Skip to content
This repository was archived by the owner on Jun 1, 2024. It is now read-only.

Rebuild Serilog.Sinks.Elasticsearch with the latest version of depend… #20

Merged
merged 5 commits into from
Jan 4, 2016

Conversation

konste
Copy link
Contributor

@konste konste commented Dec 27, 2015

…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.

…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" />
Copy link
Contributor

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.)

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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!

@nblumhardt
Copy link
Contributor

Thanks @konste - looks like CI is failing however?

@konste
Copy link
Contributor Author

konste commented Jan 4, 2016

Wow, I finally pushed it through!

@nblumhardt
Copy link
Contributor

LGTM - @mivano ?

@mivano
Copy link
Contributor

mivano commented Jan 4, 2016

Looks fine to me. 👍

@konste
Copy link
Contributor Author

konste commented Jan 4, 2016

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.

mivano added a commit that referenced this pull request Jan 4, 2016
Rebuild Serilog.Sinks.Elasticsearch with the latest version of depend…
@mivano mivano merged commit ecdf748 into serilog-contrib:master Jan 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants