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

Ship formatters in separate nuget package #208

Merged
merged 9 commits into from
Jan 27, 2019
Merged

Ship formatters in separate nuget package #208

merged 9 commits into from
Jan 27, 2019

Conversation

vhatsura
Copy link

What issue does this PR address?
#176

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

  • The commit follows our guidelines
  • Unit Tests for the changes have been added (for bug fixes / features)

Other information:

@vhatsura vhatsura changed the title Nuget/formatter Ship formatters in separate nuget package Jan 22, 2019
With higher sdk version nuget packages cannot be created locally with the following error: 'error MSB4044: The "GetPackOutputItemsTask" task was not given a value for the required parameter "PackageVersion".'
@mivano
Copy link
Contributor

mivano commented Jan 26, 2019

Thanks for the PR. Looks good, but just one question and that might just be my lack of Nuget knowledge; should the Serilog.Sinks.Elasticsearch.nuspec file not need a reference to the newly created nuget formatter package?

Next to that; can you also update the readme.md file with some details?

Thanks!

@mivano mivano mentioned this pull request Jan 26, 2019
2 tasks
@vhatsura
Copy link
Author

@mivano, you're right regarding nuspec file, however, packages are packed by dotnet pack command, which is clever and understand that Serilog.Formatting.Elasticsearch should be added to dependency graph inside nuspec file. You can check it by yourself from generated nuget package in AppVeyor.

Regarding Readme.md, I'll do it, but could clarify which info you want to see, please?

@mivano mivano merged commit b4691d6 into serilog-contrib:dev Jan 27, 2019
@mivano
Copy link
Contributor

mivano commented Jan 27, 2019

Thanks! I do indeed see the nuget packages referenced when looking in NuGet Explorer. I merged to dev and will prepare a final package.

@vhatsura
Copy link
Author

Got it, thanks. I started to write some details regarding new package. If it possible, it would be good wait changes before final package will be released.

@mivano
Copy link
Contributor

mivano commented Jan 27, 2019

Looks like the master build was skipped due to some condition. I will await your changes and will try again with those details.

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.

2 participants