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.

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

Fold S.S.C.Encoding into new System.Security.Cryptography library #61137

Merged
merged 5 commits into from
Nov 4, 2021

Conversation

bartonjs
Copy link
Member

@bartonjs bartonjs commented Nov 3, 2021

  • Split the new library from just NetCoreAppCurrent to all of the varieties that S.S.C.Encoding had.
  • Move src/libraries/System.Security.Cryptography.Encoding/src/System/... to src/libraries/System.Security.Cryptography/src/System/...
  • Move src/libraries/System.Security.Cryptography.Encoding/src/Internal/Cryptography/* to src/libraries/System.Security.Cryptography.Encoding/src/System/Security/Cryptography/*
  • Merge in the Strings.resx content that was needed.
    • Added two ObjectDisposedException.ThrowIf calls to eliminate a resource from the move.
  • Move src/libraries/System.Security.Cryptography.Encoding/tests/... to src/libraries/System.Security.Cryptography.Encoding/tests/..., with a couple files gaining the "Tests" suffix that their class had.
  • Fix the namespaces of the previously moved from-Primitives tests to match the new location.
  • Fix the namespaces of the newly moved from-Encoding tests to match the new location.
  • Make System.Security.Cryptography.Encoding ref & src just build as facades/forwarders.

Contributes to #55690.

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Nov 3, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Split the new library from just NetCoreAppCurrent to all of the varieties that S.S.C.Encoding had.
  • Move src/libraries/System.Security.Cryptography.Encoding/src/System/... to src/libraries/System.Security.Cryptography/src/System/...
  • Move src/libraries/System.Security.Cryptography.Encoding/src/Internal/Cryptography/* to src/libraries/System.Security.Cryptography.Encoding/src/System/Security/Cryptography/*
  • Merge in the Strings.resx content that was needed.
    • Added two ObjectDisposedException.ThrowIf calls to eliminate a resource from the move.
  • Move src/libraries/System.Security.Cryptography.Encoding/tests/... to src/libraries/System.Security.Cryptography.Encoding/tests/..., with a couple files gaining the "Tests" suffix that their class had.
  • Fix the namespaces of the previously moved from-Primitives tests to match the new location.
  • Fix the namespaces of the newly moved from-Encoding tests to match the new location.
  • Make System.Security.Cryptography.Encoding ref & src just build as facades/forwarders.
Author: bartonjs
Assignees: bartonjs
Labels:

area-System.Security

Milestone: 7.0.0

@bartonjs
Copy link
Member Author

bartonjs commented Nov 3, 2021

/azp list

@bartonjs
Copy link
Member Author

bartonjs commented Nov 3, 2021

@steveisok is there a way for me to trigger WASM tests from the PR (or locally)? I'm guessing this (and the next several) code move(s) will turn on more tests that are expected to fail. Seems a bit nicer if I can see them to turn them off early.

@steveisok
Copy link
Member

@steveisok is there a way for me to trigger WASM tests from the PR (or locally)?

We reactivated the full wasm interp leg on PR's, so you should get some indication.

@bartonjs
Copy link
Member Author

bartonjs commented Nov 3, 2021

We reactivated the full wasm interp leg on PR's, so you should get some indication.

Sure looks like it. Good thing Santi had feedback causing me to respin :)

@bartonjs bartonjs merged commit 8fbf206 into dotnet:main Nov 4, 2021
@bartonjs bartonjs deleted the unified_crypto_encodings branch November 4, 2021 23:34
@bartonjs bartonjs removed their assignment Nov 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants