-
Notifications
You must be signed in to change notification settings - Fork 56
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
Adding builder for VerifiableCredential #125
Conversation
Codecov Report
@@ Coverage Diff @@
## main #125 +/- ##
==========================================
+ Coverage 64.74% 66.21% +1.47%
==========================================
Files 33 32 -1
Lines 3551 3472 -79
Branches 198 197 -1
==========================================
Hits 2299 2299
+ Misses 1250 1171 -79
Partials 2 2
|
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.
Awesome to see this contribution @nitro-neal !
Looking forward to many more as you help us build out VC capabilities.
packages/credentials/tests/verifiable-credential-builder.spec.ts
Outdated
Show resolved
Hide resolved
packages/credentials/tests/verifiable-credential-builder.spec.ts
Outdated
Show resolved
Hide resolved
@mistermoe When reviewing this PR, I spotted a few things or had a few questions that I should have probably raised in the initial PR. I didn't notice them until going through a more thorough review today: 1.
|
It seems to me context needs to just be an array of objects according to the spec. In practicality though we use it as an array of strings. In the ssi-sdk we just have a new string array: contexts := []string{VerifiableCredentialsLinkedDataContext} |
@nitro-neal should we close this in favor of #148 ? |
Added VerifiableCredential builder along with unit tests.
The default constructor sets only things that are not optional:
Added the builder pattern for the rest of the fields with simple validation
Added unit tests