-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add string.GetHashCode(ROS<char>) and related APIs #20422
Add string.GetHashCode(ROS<char>) and related APIs #20422
Conversation
src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs
Outdated
Show resolved
Hide resolved
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.
Other than Jan's comments/questions, LGTM. Thanks.
Thanks @jkotas and @stephentoub! Getting to the feedback took a backseat due to some other higher-priority work, but I plan on getting back to this by end of week. |
9aa86c5
to
934dc0d
Compare
OSX failure is due to full disk, unrelated to this PR. |
@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test Edit: Or not. That's cool, too, bot. :) |
) Signed-off-by: dotnet-bot <[email protected]>
) Signed-off-by: dotnet-bot <[email protected]>
) Signed-off-by: dotnet-bot <[email protected]>
) Signed-off-by: dotnet-bot <[email protected]>
Will this get into full framework as well, or just .net core? Or is it net standard? |
Just .net core
It will make its way to net standard, but that net standard won't be supported on full framework: https://blogs.msdn.microsoft.com/dotnet/2018/11/05/announcing-net-standard-2-1/ |
Fixes https://github.com/dotnet/corefx/issues/31302.
Per https://github.com/dotnet/corefx/issues/31302#issuecomment-407542677, the newly added API on
CompareInfo
is non-virtual. Unit tests will be coming through a different PR in corefx. I'll link to that PR when it's ready.I'll also rerun some of the perf benchmarks to ensure we haven't regressed culture-aware hash code calculation performance.