-
-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Invert #if NETSTANDARD* conditional compilation conditions #1079
chore: Invert #if NETSTANDARD* conditional compilation conditions #1079
Conversation
While conceptually .NET 5, 6, 7, 8 are greater than .NET Standard 2.1 it does not apply to the `NETSTANDARD2_1_OR_GREATER` macro. So it's safer to conditionally compile on NETSTANDARD2_0 and assume that the #else branch targets a newer framework where the new bits are available in order not to use the old code path on the newest frameworks.
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I am curious, how did you run into this issue? If you consume the NuGet dependency, for example, in a net8.0
project, it will use the netstandard2.1
version. In the IDE, for instance in Rider, you can select the target framework (I think its default is netstandard2.0
).
Thank you for bringing this to my attention. It is not something I would have expected, TBH, especially considering when I first used the macro years ago (which probably was not mentioned in the MSDN back then).
Sorry I should have been more clear in my description. This is currently not an issue. This pull request is solely future-proofing the conditional compilation so that if a new target framework is added in the future (I have a plan 😉) then the correct (newest) code will be compiled. This stuff is easy to get wrong, I already fixed it in spectreconsole/spectre.console#563 too.
Yes, absolutely! This is not an issue for consumers of the Testcontainers packages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks ✌️
While conceptually .NET 5, 6, 7, 8 are greater than .NET Standard 2.1 it does not apply to the
NETSTANDARD2_1_OR_GREATER
macro.So it's safer to conditionally compile on NETSTANDARD2_0 and assume that the #else branch targets a newer framework where the new bits are available in order not to use the old code path on the newest frameworks.