Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move Sha1ForNonSecretPurposes under Common #45242

Merged
merged 5 commits into from
Nov 26, 2020
Merged

Move Sha1ForNonSecretPurposes under Common #45242

merged 5 commits into from
Nov 26, 2020

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Nov 25, 2020

Avoid code duplication and contribute to #45237 and #45032

@jkotas
Move Sha1ForNonSecretPurposes under Common
0a74220 
Avoid code duplication and contribute to dotnet#45237 and dotnet#45032
GrabYourPitchforks
GrabYourPitchforks approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@GrabYourPitchforks GrabYourPitchforks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Straightforward enough :)

@GrabYourPitchforks
Copy link
Member

No harm in duplicating this line as an internal helper within this file.

runtime/src/libraries/System.Private.CoreLib/src/System/Numerics/BitOperations.cs

Lines 418 to 419 in fe80a77

public static uint RotateLeft(uint value, int offset)
=> (value << offset) | (value >> (32 - offset));

@jkotas
Copy link
Member Author

jkotas commented Nov 26, 2020

The OOB library has BitOperations stub for this and other reasons. It just was not in the right namespace to be picked up here.

@GrabYourPitchforks
Copy link
Member

GrabYourPitchforks commented Nov 26, 2020 •
edited
Loading

Looks like System.Net.WebSockets.WebSocketProtocol.csproj still references BitOperations.cs? Removing the ifdefs could break that project.

Nevermind - they're referencing their own copy, not the CoreLib copy.

@GrabYourPitchforks
Copy link
Member

Unit test failure might be known issue #24181?

@runfoapp runfoapp bot mentioned this pull request Nov 26, 2020
Closed
@jkotas jkotas merged commit bf1b48d into dotnet:master Nov 26, 2020
@jkotas jkotas deleted the Sha1ForNonSecretPurposes branch November 26, 2020 16:25
@ghost ghost locked as resolved and limited conversation to collaborators Dec 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marek-safar marek-safar marek-safar left review comments

@GrabYourPitchforks GrabYourPitchforks GrabYourPitchforks approved these changes

Assignees
No one assigned
Labels
area-System.Runtime
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jkotas @GrabYourPitchforks @marek-safar @Dotnet-GitSync-Bot