-
Notifications
You must be signed in to change notification settings - Fork 8
Update language around the defined types and their usages #186
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
Conversation
As discussed on the call today I've updated My understanding -- and please correct me otherwise -- is that to make this work in LD-land there needs to be a definition in a context file that adds a property to "jsonSchema": {
"id": "https://www.w3.org/TR/vc-json-schema#jsonSchema"
"type": "@json"
} which should go here. But it seems like then the data model would need some text on the I would like to avoid this specification shipping its own context which will then need to be included with each VC that intends to use this specification....is there another way to accomplish this? @msporny @OR13 |
index.html
Outdated
@@ -301,7 +322,7 @@ <h3>VerifiableCredentialSchema2023</h3> | |||
"type": ["VerifiableCredential", "VerifiableCredentialSchema2023"], | |||
"issuer": "https://example.com/issuers/14", | |||
"issuanceDate": "2010-01-01T19:23:24Z", | |||
"credentialSubject": { | |||
"jsonSchema": { |
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.
-1 to this approach.
The credentialSubject
property is required in the VCDM, so removing results in an object that isn't a VC.
As an alternative, I suggest taking the same approach as the one taken in StatusList2021Credential. That is, make credentialSubject.type
mandatory. An example of my suggestion below. This would make the vc graph much more palatable, and I believe this uses the more typical JSON-LD patterns.
{
"@context": [
"https://www.w3.org/ns/credentials/v2",
"https://www.w3.org/ns/credentials/examples/v2"
],
"id": "https://example.com/credentials/3734",
"type": ["VerifiableCredential", "VerifiableCredentialSchema2023"],
"issuer": "https://example.com/issuers/14",
"issuanceDate": "2010-01-01T19:23:24Z",
"credentialSubject": {
"id": "https://example.com/credentials/schema/12",
"type": "JsonSchema2023",
"jsonSchema": {
"$id": "https://example.com/schemas/email-credential-schema.json",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"name": "EmailCredential",
"description": "EmailCredential using VerifiableCredentialSchema2023",
"type": "object",
"properties": {
"credentialSubject": {
"type": "object",
"properties": {
"emailAddress": {
"type": "string",
"format": "email"
}
},
"required": ["emailAddress"]
}
}
}
}
}
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.
Yes, to what @andresuribe87 said above. That's a better way to do it.
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 suggestion! thank you
@andresuribe87 proposes something cleaner above. Given @andresuribe87 example... you're probably better off renaming
I expect that might get push back from the WG. I suggest that @andresuribe87's approach is a better design.
No, the vocabulary could just point to your JSON Schema spec... we wouldn't need to document anything in vc-data-model.
Yes, put your definition in the core VC context (so you don't need to ship your own vocabulary/context), triggered by type... so, something like this:
Then your example becomes:
|
thank you @andresuribe87 @msporny I've updated the PR with your suggestions. I also included the term definition for Follow-up PR for the VCDM w3c/vc-data-model#1215 |
Once merged, not only the |
Co-authored-by: Manu Sporny <msporny@digitalbazaar.com>
thanks @iherman I'm happy to transfer ownership of w3c/vc-data-model#1215 to you after this and 1209 are merged |
@msporny @OR13 @andresuribe87 please take a look and see if you're ready to approve |
} | ||
} | ||
</pre> | ||
</p> | ||
<p> | ||
<p> | ||
Upon dereferencing the value of the <code>id</code> <code>https://example.com/credentials/3734</code>, | ||
a process also be referred to as <a>schema resolution</a>, the following verifiable credential, |
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.
Why call out the schema resolution
as a separate process? Isn't it a simple URL fetch?
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.
it is but it's a necessary step in using the spec, so it can be given a name which would be useful in enumerating the steps to process like
- get credential
- perform schema resolution
- evaluate schema against credential
Fix #172 #178
I believe this needs a change to the core vocab to add a new property
jsonSchema
with thetype
as@json
Preview | Diff