-
Notifications
You must be signed in to change notification settings - Fork 772
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
[sdk] Only use BCL HashCode for .NET 6+ #4362
Conversation
Is this considered as a breaking change or not (from .NET team's perspective)? |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4362 +/- ##
==========================================
+ Coverage 84.79% 84.81% +0.01%
==========================================
Files 300 300
Lines 12065 12065
==========================================
+ Hits 10231 10233 +2
+ Misses 1834 1832 -2
|
The PR title is "Help compiler enforce nullability annotations" not anything about changing |
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.
This PR has me wondering if it would just make sense to remove all NETSTANDARD2_1
preprocessor directives.
I think in some situations this will mean that certain perf improvements will not be available to netstandard2.1 builds, but I think that's ok.
I don't think I feel the same way about NETSTANDARD2_0 directives because netstandard2.0 serves folks sharing code between .NET Framework and modern .NET.
Relates to #4204
Changes
System.HashCode
so it is only used on .NET6+ runtimes.Details
We were talking about metrics not working on .NET Core 3.1 (#4204) today on the SIG. The issue seems to be differences around
null
handling inHashCode
, which is very strange. I did some digging on it.It seems like this PR accidentally introduced a breaking logical change in
HashCode
.What this means is .NET Standard 2.1 and .NET6+ versions of
HashCode
have logical differences. To be safe and avoid any issues I thought it best just to use our fallback logic for .NET Standard 2.1.PS: There is an out-of-band NuGet version of HashCode. I tried all versions of it, none of them have the
null
logic updated to match dotnet/runtime current state.Merge requirement checklist
CHANGELOG.md
files updated for non-trivial changes