-
Notifications
You must be signed in to change notification settings - Fork 192
Rebuild Serilog.Sinks.Elasticsearch with the latest version of depend… #20
Conversation
…encies to avoid the need for Serilog binding redirects in each project consuming this sink.
</dependencies> | ||
</metadata> | ||
<files> | ||
<file src="bin40\Release\Serilog.Sinks.Elasticsearch.dll" target="lib\net40" /> | ||
<file src="bin40\Release\Serilog.Sinks.Elasticsearch.xml" target="lib\net40" /> | ||
<file src="bin\Release\Serilog.Sinks.Elasticsearch.dll" target="lib\net45" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropping support for .NET 4? (The 4.5 binaries are included in the package by default.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not sure how exactly support for net40 is done. Are you saying those two lines should stay as they were? My concern is that the sink's own packages.config (there is only one) always refers to net45 version of Serilog and SerilogFullNetFx, so it seems like what this sink builds as net40 still requires 4.5!
In short - it seems to me that Serilog.Sinks.Elasticsearch already dropped net40 silently. I may be wrong though.
Do you know what specific nuget command is used to build the package with both binaries? I don't see it among the sources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just discovered Build.ps1. Taking another look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, those two lines are reverted. Thanks for catching that quickly!
Thanks @konste - looks like CI is failing however? |
Wow, I finally pushed it through! |
LGTM - @mivano ? |
Looks fine to me. 👍 |
Great! Pull it, please! I'm so tired of keeping those pesky binding redirects from Serilog 1.4 to 1.5 in about 50 projects only for that sink. |
Rebuild Serilog.Sinks.Elasticsearch with the latest version of depend…
…encies to avoid the need for Serilog binding redirects in each project consuming this sink.
This change should help numerous build issues we have with Serilog.Sink.Elasticsearch and serve as a foundation for coming improvements in ES response handling.