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

refactor: ledger & anoncreds typing, part 3 #1124

Merged
merged 43 commits into from
Feb 16, 2024
Merged

refactor: ledger & anoncreds typing, part 3 #1124

merged 43 commits into from
Feb 16, 2024

Conversation

mirgee
Copy link
Contributor

@mirgee mirgee commented Feb 8, 2024 •
edited
Loading

Continuation of #1116 and #1118.

The changes in 913f348 are breaking, so may need to be (partially) reverted.

@codecov-commenter
Copy link

codecov-commenter commented Feb 8, 2024 •
edited
Loading

Codecov Report

Attention: 315 lines in your changes are missing coverage. Please review.

Comparison is base (6a8e0f7) 0.05% compared to head (9dc0335) 0.05%.

Files Patch % Lines
...e/src/anoncreds/credx_anoncreds/type_conversion.rs 0.00% 88 Missing ⚠️
aries/aries_vcx/tests/test_verifier.rs 0.00% 45 Missing ⚠️
...ries_vcx_core/src/anoncreds/credx_anoncreds/mod.rs 0.00% 38 Missing ⚠️
...ies/misc/test_utils/src/mockdata/mock_anoncreds.rs 0.00% 30 Missing ⚠️
...es_vcx/src/common/proofs/prover/prover_internal.rs 0.00% 26 Missing ⚠️
...vcx/src/protocols/issuance/issuer/state_machine.rs 0.00% 11 Missing ⚠️
..._types/src/data_types/messages/revocation_state.rs 0.00% 10 Missing ⚠️
...vcx/src/protocols/issuance/holder/state_machine.rs 0.00% 7 Missing ⚠️
aries/aries_vcx/src/common/proofs/prover/mod.rs 0.00% 6 Missing ⚠️
aries/aries_vcx/src/common/proofs/verifier/mod.rs 0.00% 6 Missing ⚠️
... and 18 more
Additional details and impacted files 
@@           Coverage Diff            @@
##            main   #1124      +/-   ##
========================================
- Coverage   0.05%   0.05%   -0.01%     
========================================
  Files        495     497       +2     
  Lines      25124   25298     +174     
  Branches    4535    4597      +62     
========================================
  Hits          13      13              
- Misses     25111   25285     +174
Flag Coverage Δ
unittests-aries-vcx 0.05% <0.00%> (-0.01%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mirgee mirgee force-pushed the anoncreds-types-2 branch 6 times, most recently from 0ff547e to 44b6f94 Compare February 9, 2024 11:17
@mirgee mirgee added the breaking label Feb 9, 2024
@mirgee mirgee force-pushed the anoncreds-types-2 branch 2 times, most recently from d314493 to 913f348 Compare February 9, 2024 12:06
@Patrik-Stas Patrik-Stas mentioned this pull request Feb 9, 2024
Closed
@mirgee mirgee force-pushed the anoncreds-types-2 branch from 913f348 to 3d71ebd Compare February 9, 2024 20:00
@mirgee mirgee force-pushed the anoncreds-types-1 branch from e7325df to 0a54b22 Compare February 9, 2024 20:01
@mirgee mirgee force-pushed the anoncreds-types-2 branch from 3d71ebd to 92c7c43 Compare February 9, 2024 20:02
@mirgee mirgee marked this pull request as ready for review February 9, 2024 20:53
@mirgee mirgee requested a review from Patrik-Stas February 9, 2024 20:53
Base automatically changed from anoncreds-types-1 to main February 9, 2024 23:59
@mirgee mirgee force-pushed the anoncreds-types-2 branch 3 times, most recently from 1160c2c to 33cdd19 Compare February 13, 2024 09:19
@mirgee mirgee changed the base branch from main to just February 13, 2024 09:19
@mirgee mirgee force-pushed the just branch 3 times, most recently from 9946240 to 41d4a6a Compare February 13, 2024 13:31
@mirgee mirgee force-pushed the anoncreds-types-2 branch 3 times, most recently from a29d3d8 to 2e531df Compare February 13, 2024 15:00
mirgee added 14 commits February 14, 2024 23:09
@mirgee
Revert "retrieved -> selected"
7554fbc 
This reverts commit 7adf37c.

Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
_make_cred_info returns typed result
8bfb69c 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
get_credentials returns typed result
c88b117 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Type RevocationState
60943f1 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Type alias for credential id
0562c32 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Type alias for link secret id
640c694 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
The build_rev_states_json return value
f9991cc 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Type revocation states map
bb0098e 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Use type aliases for complex maps
ce78863 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Type credential definition config
8155a84 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Credential defintion config
d6a2464 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Schema attribute names
dc25b47 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Simplify some anoncreds conversions
33fb06a 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee
Simplify some credx conversions
27233f9 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
Patrik-Stas
Patrik-Stas requested changes Feb 15, 2024
View reviewed changes
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.

awesome stuff 👍 But I am in favor of keeping this non-breaking and doing these other, desirable but breaking changes elsewhere (in particular those de/serialization breaking changes)

.github/workflows/main.yml Outdated Show resolved Hide resolved
aries/aries_vcx/src/common/credentials/mod.rs Outdated Show resolved Hide resolved
aries/aries_vcx/src/common/primitives/credential_definition.rs Outdated Show resolved Hide resolved
aries/aries_vcx/src/common/primitives/credential_schema.rs Show resolved Hide resolved
aries/aries_vcx/src/common/primitives/credential_schema.rs Outdated Show resolved Hide resolved
aries/aries_vcx/src/common/proofs/prover/mod.rs Show resolved Hide resolved
@mirgee mirgee closed this Feb 15, 2024
@mirgee mirgee reopened this Feb 15, 2024
@mirgee
Address code review comments
9107a0f 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
Patrik-Stas
Patrik-Stas previously approved these changes Feb 15, 2024
View reviewed changes
@Patrik-Stas
Copy link
Contributor

Patrik-Stas commented Feb 15, 2024 •
edited
Loading

Just realized that you have already reverted the breaking changes, it seems. If you can confirm (if so, also please remove the breaking label if that's the case) I am not missing something, I think we can merge

@mirgee
Make cred_rev_id in RevocationInfoV1 back to string
9dc0335 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee mirgee removed the breaking label Feb 15, 2024
@mirgee
Copy link
Contributor Author

mirgee commented Feb 15, 2024

The breaking changes were reverted in 9dc0335 (u32 json-(de)serializes differently from String)

@mirgee mirgee requested a review from Patrik-Stas February 15, 2024 15:05
@Patrik-Stas Patrik-Stas merged commit ec0df29 into main Feb 16, 2024
29 checks passed
@Patrik-Stas Patrik-Stas deleted the anoncreds-types-2 branch February 16, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Patrik-Stas Patrik-Stas Patrik-Stas approved these changes

Assignees
No one assigned
Labels
refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mirgee @codecov-commenter @Patrik-Stas