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

Make PeerDid generic over numalgo #887

Merged
merged 2 commits into from
Jul 3, 2023
Merged

Make PeerDid generic over numalgo #887

merged 2 commits into from
Jul 3, 2023

Conversation

mirgee
Copy link
Contributor

@mirgee mirgee commented Jun 27, 2023

Makes the peer did submethod (numalgo) part of the PeerDid type, allowing for statically ensuring

  • its properties (e.g. resolvable to a DDO, ability to be shortened to did:peer:3, etc.) or
  • concrete value of the numalgo underlying the peer did.

GenericPeerDid enum was added to cover situations when there is no concrete numalgo expected.

@mirgee mirgee force-pushed the refactor/did-peer branch 3 times, most recently from d75d554 to 8d1e713 Compare June 27, 2023 13:21
Base automatically changed from feature/did-peer-3 to main June 27, 2023 13:21
@mirgee mirgee force-pushed the refactor/did-peer branch 5 times, most recently from b357d56 to 6813c70 Compare June 28, 2023 06:26
@mirgee mirgee marked this pull request as ready for review June 28, 2023 06:46
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2023

Codecov Report

Merging #887 (0114516) into main (c1180a7) will decrease coverage by 3.66%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #887      +/-   ##
==========================================
- Coverage   49.90%   46.25%   -3.66%     
==========================================
  Files         432      432              
  Lines       35058    35058              
  Branches     7617     7611       -6     
==========================================
- Hits        17497    16215    -1282     
- Misses      12284    13900    +1616     
+ Partials     5277     4943     -334     
Flag Coverage Δ
unittests-aries-vcx 46.24% <ø> (-3.64%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 53 files with indirect coverage changes

Signed-off-by: Miroslav Kovar <[email protected]>
Patrik-Stas
Patrik-Stas previously approved these changes Jun 29, 2023
Copy link
Contributor

@Patrik-Stas Patrik-Stas left a comment

Choose a reason for hiding this comment

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

left minor comment, generally lgtm

did_peer/src/peer_did/peer_did/generic.rs Show resolved Hide resolved
did_peer/src/peer_did/peer_did/generic.rs Show resolved Hide resolved
Copy link
Contributor

@bobozaur bobozaur left a comment

Choose a reason for hiding this comment

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

Generally LGTM, but I left a few comments with my opinions.

did_peer/src/peer_did/numalgos/traits.rs Outdated Show resolved Hide resolved
did_peer/src/peer_did/peer_did/mod.rs Show resolved Hide resolved
@Patrik-Stas Patrik-Stas merged commit c49c1ab into main Jul 3, 2023
39 of 40 checks passed
@Patrik-Stas Patrik-Stas deleted the refactor/did-peer branch July 3, 2023 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants