castleproject / Core

Castle Core, including Castle DynamicProxy, Logging Services and DictionaryAdapter
http://www.castleproject.org/
Other
2.2k stars 467 forks source link

Stop testing for .NET Core 2.1/3.1 and update some nuget packages #660

Closed Romfos closed 1 year ago

Romfos commented 1 year ago

Motivation: Soon .NET 8 will be released and this PR is part of preparation for this. Small cleanup of build\test system

Changes:

This pr do not change product by itself. Only test\build infrastructure. No need to release new nuget package version

.Notes: .NET Core 2.1 is out of support https://devblogs.microsoft.com/dotnet/net-core-2-1-will-reach-end-of-support-on-august-21-2021/

.NET Core 3.1 is out of support https://devblogs.microsoft.com/dotnet/net-core-3-1-will-reach-end-of-support-on-december-13-2022/

stakx commented 1 year ago

I tend to agree that testing on these now-unsupported platforms, by itself, isn't that important anymore... however, since this library targets net462, net6.0, but also netstandard2.0 and netstandard2.1, we need to figure out what the last two TFMs mean for running tests.

IIRC, it is for the .NET Standard TFMs that we're currently testing on .NET Core 2.1 and 3.1.

Given that .NET 5 has started moving away from .NET Standard starting with version 5 (see the official docs), one might wonder why we are even keeping these TFMs around at all. .NET 4.6.2 and 6 (the two other TFMs) already cover .NET Standard 2.x, and we're not testing our library for any other platforms that support it (Xamarin, UWP, Unity, etc.); Mono being the sole exception, but that falls under the net462 umbrella.

The main reason to keep targeting netstandard2.x is perhaps because downstream libraries do so, too, and maybe we do not want to force them into abandoning those TFMs?

I suppose dropping netstandard2.x support might be the technically accurate / honest thing to do, but it might be an unpopular decision. And if we're keeping those TFMs around, perhaps we should also test them in some way... or are we OK with declaring .NET Standard 2.x support without testing it at all?

Maybe I'm overthinking this... any opinions? @jonorossi?

Romfos commented 1 year ago

Yea, we are king of in the middle of evolution .NET ecosystem. Some of .NET Standard platforms are unsupported, some are still supported I agree that we cannot drop just .NET Standard 2.0 from TFM list now and we need to have at least minimal testing for it

At the same I think that make sense start thinking about dropping .NET Standard 2.1

jonorossi commented 1 year ago

@stakx I think you've got it right.

Microsoft has the following recommendations for when to keep using .NET Standard, and that's for libraries that want to share between .NET Framework and .NET [Core], which is still useful for libraries that depend on interfaces defined in Castle.Core:

.NET Standard not deprecated

.NET Standard is still needed for libraries that can be used by multiple .NET implementations. We recommend you target .NET Standard in the following scenarios:

  • Use netstandard2.0 to share code between .NET Framework and all other implementations of .NET.
  • ...

I agree we should only run unit tests on supported .NET platforms, and the build warnings need to be cleaned up, e.g.:

.NET Core 2.1 is no longer supported and will not receive security updates in the future. (https://github.com/castleproject/Core/actions/runs/5998689016)

stakx commented 1 year ago

@jonorossi, cleaning up the build warnings would be easy enough by simply suppressing them (I'll look into it!), but are you suggesting we go further and drop the netcoreapp* targets for our test projects, while keeping the netstandard* TFMs in the library? That would leave the netstandard* assemblies untested since we have no other runtime we could use for them. (I could probably live with that, esp. because we have no conditional compilation that distinguishes them from the net6 assembly.)

jonorossi commented 10 months ago

@stakx I think best to come back to this after your list of areas to address post the next release. Some parts of the library that'll get pulled out could just target .NET Standard only.