serilog-sinks-console
serilog-sinks-console copied to clipboard
Add net6.0 Target
What issue does this PR address? Please add net6.0, net7.0 targets
Does this PR introduce a breaking change? No
Please check if the PR fulfills these requirements
- [yes ] The commit follows our guidelines
- [ n/a] Unit Tests for the changes have been added (for bug fixes / features)
Other information: Please list any other relevant information here
while i dont object to this PR. i am curious as to why it is required. targeting net5 should be enough to support net6 and net7
See also: https://github.com/serilog/serilog-sinks-file/pull/295
I think this in some ways comes down to a support issue; we do hit reports pretty much every week about this or similar concerns.
Serilog 3 is now targeting:
<TargetFrameworks Condition=" '$(OS)' == 'Windows_NT'">net462;net471</TargetFrameworks>
<TargetFrameworks>$(TargetFrameworks);netstandard2.1;netstandard2.0;net5.0;net6.0;net7.0</TargetFrameworks>
It might make the most sense to a) target these TFMs here (and in the other linked PR), b) bump the major package version to communicate the breaking change (dropping the oldest .NET Standard versions), and c) move to enshrine these in a standard .props file we share across all of the core Serilog packages.
(Edit: I should say - except for the Extensions ones, which target .NET versions 1:1 for other reasons.)
What do you think @SimonCropp @bartelink ?
I don't see how this ever becomes manageable unless we figure out a unified policy and have a statement to refer people to.
I'd say the basics are:
- there are cases where APIs / overloads added in newer TFMs enable things to be done more efficiently. We're all for taking advantage of that - if doing a variant targeting a specific TFM brings value
- net6.0 feels like a reasonable place to draw a line; net5.0 was not an LTS release so will get people worrying unnecessarily.
Sinks.Fileshould get an upgrade as a result. - doing a custom net7.0 and net8.0 just bloats the package unless there's an actual new API being used. It also invites busywork the minute the net9.0 betas hit the street (not far off)
- if there is a feature or perf need that could benefit from a net9.0 feature, we can add a dependency on it
- if that happened, we can put in net10.0 alongside, and trim the net9.0 when the next major version rolls
- we would not add net11.0 or net12.0 unless something actually needed and used a new API
In summary, my proposed policy for impls is:
- anything that needed net5.0 triggers adding net6.0 when someone wants (NOTE this does not actually solve any problem)
- when we roll a new major version
- netstandard3.1, net45 and friends are eligible for removal unless a speciic package is providing downlevel support without any problems
- netstandard2.0 always stays if it is supported
- all non LTS requirements are replaced with the current LTS
- so a new version now will move netstandard2.1, netcoreapp3.1, net5.0 to net6.0
- a new version when net8.0 is released will replace net5.0 with net6.0, net7.0 with net8.0
- min LTS requirement is not upgraded unless there is a reason (we have code that takes advantage of it)
- if something targets net6.0, we don't remove that or add net8.0 or net10.0 or net12.0 just because net12.0 is the current LTS
Then we need specific rules about versioning:
- the tests in
devshould target all released FWs (i.e. definitelynet7.0) - there comes a point when one almost released .NET version should also be eligible (e.g., targeting
net8.0now might be reasonable even if it's not actually released).net9.0should not be considered until at least afternet8.0is released - the tests in
mainshould not be expected to target EOL'd TFMs- so net5.0 is a gonner - replace with net6.0
- net7.0 is eligible to add
- net7.0 is eligible for removal when net8.0 is released
- Q: can net8.0 be added before it's released? I assume if there are interesting changes there will be times when we want to do this. Probably no need to set an actual timeline though
- at the point where we have sufficient compelling justification for a breaking change, we roll a new version
- remove Obsolete methods
- update all test dependencies (always allowed even without a major release, but forced by a major release)
- TFMs for implementation projects should be reassessed when a new version is introduced
- any
net3/5should be removed and replaced withnet6.0(but removal always needs to be a major version bump) - perhaps trim
netstandard< 2.0 ? - are we trimming
net45? (File has a variant for it) - no point trimming
netstandard2.0 netstandard2.1does not have many users - should consider removing in favor of anet6.0dep
- any
- sometimes nothing has happened in a repo for ages. That should be considered normal for a Sink repo. Perhaps a prominent link to the download stats might a) give people an idea of just how much usage something has, as a way to inspire confidence b) make them appreciate that SO it the place to ask questions, rather than the issue tracker
Perhaps Nick could add a VERSIONING.md skeleton to Serilog core repo and we define the policy there, including a notes sections for warts that are in place but deprecated
So, specific things now:
- roll a new
Serilog.Sinks.Fileversion
- atm impl targets
net45;netstandard1.3;netstandard2.0;netstandard2.1;net5.0 - 👍 could do a 5.0.1 that adds
net6.0to bring it into line with policy of replacingnetstandard2.1,netcoreapp3.1,net5.0withnet6.0- a similar PR for
net7.0(or 9/11 when released) should be refused as not LTS (and no new APIs that enable anything) - I also think adding
net8.0should not be explicitly targeted (even when net6.0 EOLs)
- a similar PR for
- 👎 could do a 6.0 that removes
net5.0andnetstandard2.1targeting, but there is nothing compelling and new ton justify it, and it's not broken.- net45 drives conditionals so ideally that can be removed to simplify things
- would consider targeting
netstandard2.0only, unless there is a reason and value in supportingnetstandard1.3(or2.1or5.0). If we get that wrong, new ones can always be re-added in a6.1
- tests target
net48;net5.0- no need to keep targeting
net5.0; that should change tonet6.0. Can addnet7.0, though (no restriction on non-LTS). - (See above, maybe add net8.0 even now, depending on whether we wait for RTM)
net48makes sense as long as there isnetstandard2.0(ornet452ornetstandard1.3)
- no need to keep targeting
- new
Serilog.Sinks.Consolerelease
- targets `net45;netstandard1.3;netstandard2.0;net5.0
- tests target
netcoreapp2.1;netcoreapp2.2;netcoreapp3.0;netcoreapp3.1;net452;net462;net472;net48;net5.0 net5.0opts in to usingspan- tests should target net48, net6.0 (and optionally net7.0, maybe net8.0)
- impl should add a
net6.0target in line with policy above - 👍 should add a 4.2.0 that has a
net6.0variant in line with above policy of not targetingnetstandard2.1/netcoreapp3/net5.0 - 👎 no compelling reason for a 5.0.0, but if there was
- remove obsoletions
- stop targeting
net5.0(need to provide anet6.0as it is actually being used)
addendum: see also this comment for more reasons to have a VERSIONING.md https://github.com/serilog/serilog-sinks-console/pull/145#discussion_r1318737211
@nblumhardt I guess it's too late now, but maybe a 5.0.1 that trims the netstandard2.1, net5.0 and net7.0 targets might still make sense? https://github.com/serilog/serilog-sinks-console/pull/145#pullrequestreview-1691901895
@bartelink thanks for the ping :+1:
I think the packages that just went out all align with the same TFMs as Serilog 3.1, definitely room for an update but not sure yet whether we should call that "3.2" or "4.0" ----- brewing some ideas :-)
I think the packages that just went out all align with the same TFMs as Serilog 3.1,
True; I guess it's not a massive deal
For avoidance of doubt,Serilog.Sinks.File has not got the net7.0 added (and has not been released for a while; adding a net6.0 is the key thing a push will add). I'd suggest we don't add net7.0 (or anything) in the 5.x timeline. If we're looking to do a File 6.0.0, rather than a 5.1/5.0.1 then likely dropping netstandard2.1;net5.0 would make sense too at that point.
Sounds good 👍
Was this superceeded by #157 ?
Thanks @zachrybaker and reviewers; everything should now be aligned with Serilog 4's TFMs, we got there in the end! :-)
Huzzah!
On Fri, Jun 21, 2024 at 4:05 AM Nicholas Blumhardt @.***> wrote:
Thanks @zachrybaker https://github.com/zachrybaker and reviewers; everything should now be aligned with Serilog 4's TFMs, we got there in the end! :-)
— Reply to this email directly, view it on GitHub https://github.com/serilog/serilog-sinks-console/pull/145#issuecomment-2182335561, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACDSMSFLLEIDTQPKW5IEVTZIPUFBAVCNFSM6AAAAABJUV5V7SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBSGMZTKNJWGE . You are receiving this because you were mentioned.Message ID: @.***>