search

dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.7k stars 1.06k forks source link

Package authors can use #if to perform version checks #14540

Closed terrajobst closed 3 years ago

terrajobst commented 3 years ago

Spec

omajid commented 3 years ago

This snippet is very confusing to me:

void M()
{
    #if NETFRAMEWORK461_OR_GREATER
        LegacyNetFxBehavior();
    #elif NETCOREAPP3_1_OR_GREATER
        LegacyNetCoreBehavior();
    #elif NET6_0_OR_GREATER
        Net6OrHigherBehavior();
    #else
        #error Unhandled TFM
    #endif
}

It seems NETCOREAPP3_1_OR_GREATER means ".NET Core 3.1 or .NET 5 but not .NET 6"? This seems very strange. .NET 6 is ".NET Core 3.1 or later" from how I am looking at it.

I would have expected it to be written like this, so the highest version check comes first.

void M()
{
    #if NET6_0_OR_GREATER
        Net6OrHigherBehavior();
    #elif NETCOREAPP3_1_OR_GREATER
        LegacyNetCoreBehavior();
    #elif NETFRAMEWORK461_OR_GREATER
        LegacyNetFxBehavior();
    #else
        #error Unhandled TFM
    #endif
}
terrajobst commented 3 years ago

It seems NETCOREAPP3_1_OR_GREATER means ".NET Core 3.1 or .NET 5 but not .NET 6"? This seems very strange. .NET 6 is ".NET Core 3.1 or later" from how I am looking at it.

You're totally correct. The order in my if-statements is totally wrong.

terrajobst commented 3 years ago

@omajid You may want to read the spec now and leave feedback there.

omajid commented 3 years ago

From a quick look, the new spec looks good. Thanks for fixing it so quickly!

marcpopMSFT commented 3 years ago

Wait to implement until https://github.com/dotnet/designs/pull/165 has been decided on or not.

terrajobst commented 3 years ago

Based on community feedback I decided to abandon that PR and leave it as OR_GREATER. @sfoslund, you're good to go!

sfoslund commented 3 years ago

Fixed by #14798, merged into 5.0.2xx

chucker commented 3 years ago

I want to conditionally implement IAsyncDisposable when available. Am I correct to assume that this is the right way of going about it?

#if NETSTANDARD2_1 || NETCOREAPP3_0 || NETCOREAPP3_1 || NET5_0 || NETCOREAPP3_0_OR_GREATER
    public partial class MyClass : IAsyncDisposable
    {
        public async ValueTask DisposeAsync()
        {
            // impl
        }
    }
#endif

My thinking on this is that .NET Core 3.0 and 3.1 and .NET 5 didn't actually ship with this change, so people may have an SDK that doesn't support it.