-
Notifications
You must be signed in to change notification settings - Fork 204
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
feat: Add cheqd-sdk module #1334
Conversation
Signed-off-by: DaevMithran <[email protected]>
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.
Left an initial review. Really like the addition, this is great, but have some questions/concers about integrating with existing AFJ infrastructure and making sure it works in multitenancy/paralel processing cases
packages/cheqd-sdk/src/anoncreds/services/CheqdSdkAnonCredsRegistry.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: DaevMithran <[email protected]>
Signed-off-by: DaevMithran <[email protected]>
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.
Some final changes. Have you tested these changes in React Native? We had to apply a lot of workarounds to get it to work, and we should probalby add documentation for this.
packages/cheqd-sdk/src/anoncreds/services/CheqdAnonCredsRegistry.ts
Outdated
Show resolved
Hide resolved
packages/cheqd-sdk/src/anoncreds/services/CheqdAnonCredsRegistry.ts
Outdated
Show resolved
Hide resolved
packages/cheqd-sdk/src/anoncreds/services/CheqdAnonCredsRegistry.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: DaevMithran <[email protected]>
92b8312
to
9a5c107
Compare
Signed-off-by: DaevMithran <[email protected]>
58e459a
to
f319239
Compare
Signed-off-by: DaevMithran <[email protected]>
Signed-off-by: DaevMithran <[email protected]>
90c649c
to
1f01dcb
Compare
chore: some fixes Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: DaevMithran <[email protected]>
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.
Great Job! A lot of stuff are done, let's maybe chat about possible changes if you want.
Signed-off-by: DaevMithran <[email protected]>
@DaevMithran Is the PR ready to be merged, or do you still need to incoroporate any of the outstanding comments? Also, it seems DCO is failing. |
Signed-off-by: DaevMithran <[email protected]>
d7ff6f6
to
c831a97
Compare
@TimoGlastra fixed the DCO, it's ready to be merged. Just one thing should we store ledger responses in cache? I haven't included that yet. |
Signed-off-by: DaevMithran <[email protected]>
I think we can add caching in a follow-up PR |
Codecov Report
@@ Coverage Diff @@
## main #1334 +/- ##
==========================================
- Coverage 85.86% 85.61% -0.25%
==========================================
Files 870 884 +14
Lines 20489 21032 +543
Branches 3449 3558 +109
==========================================
+ Hits 17592 18007 +415
- Misses 2725 2851 +126
- Partials 172 174 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: DaevMithran <[email protected]>
2ef6c93
to
3eb7517
Compare
@TimoGlastra The tests failing are not specific to this PR, its the same one's failing in main as well |
1e6ae48
to
18e0fdf
Compare
Signed-off-by: DaevMithran <[email protected]>
18e0fdf
to
6414422
Compare
This PR adds cheqd-sdk module in AFJ with the following interfaces
Added tests for all the classes