Serilog 4 updates; unpublish enricher types that are already accessible via extensions #63
Conversation
Build.ps1
Outdated
|
|
||
| if($suffix) { | ||
| & dotnet pack -c Release --include-source --no-build -o ../../artifacts --version-suffix=$suffix | ||
| & dotnet build -c Release --version-suffix=$buildSuffix -p:EnableSourceLink=true |
There was a problem hiding this comment.
Should we turn on ContinuousIntegrationBuild whilst making changes?
(e.g. https://github.com/serilog/serilog/blob/fdf4a4872b4effd02dcd7d788e45b6b8ffdfa2fc/Build.ps1#L20)
Should classes which have been changed from public to internal be sealed if there are no subclasses? |
Build.ps1
Outdated
|
|
||
| if($suffix) { | ||
| & dotnet pack -c Release --include-source --no-build -o ../../artifacts --version-suffix=$suffix | ||
| & dotnet build -c Release --version-suffix=$buildSuffix -p:EnableSourceLink=true |
There was a problem hiding this comment.
A general question about SourceLink support in the enrichers -
As it stands, this package appears to have neither embedded PDBs nor a .snupkg file.
Compare that with the Process enricher, which has embedded PDBs inside the .nupkg.
Should they all be changed to do one or the other?
|
Thanks for the review @Numpsy! I've checked in on the current thinking around symbol packaging, seems like SNUPKGs are the way to go for us, I've tweaked the build setup to publish them. It'd be nice to propagate this back across the other Serilog projects, I'll aim to do that as I work through the others. As of .NET 8, SourceLink shouldn't need any configuration or extra packages for GitHub projects, so I've dropped off the explicit flag. Marked the internal types |
I'm on my way through all of the Serilog projects, updating to pull in and align with Serilog 4, and since this is a major version bump, taking the opportunity to consider breaking changes that we might not make otherwise. I'm hoping to get our many packages into a fresh and easily-maintainable state.
This PR aligns the package with the Serilog 4 TFMs (dropping anything pre-
netstandard2.0).I think we should also take the opportunity to internalize the implementation types that effectively duplicate the API of the extension methods. The public API surface of this package has grown quite large, but the
Enrich.With*methods cover all of it and are the way users should interact with this functionality (in a similar vein, all of the Serilog org sink types are now internal, too).