search

serilog / serilog-enrichers-environment

Enrich Serilog log events with properties from System.Environment.
Apache License 2.0
80 stars 33 forks source link

Serilog 4 updates; unpublish enricher types that are already accessible via extensions #63

Closed nblumhardt closed 4 months ago

nblumhardt commented 5 months ago

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

Numpsy commented 5 months ago

I think we should also take the opportunity to internalize the implementation types

Should classes which have been changed from public to internal be sealed if there are no subclasses?

nblumhardt commented 5 months ago

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 sealed where possible 👍