add vc-json-schema to vocab and context #1178
Conversation
contexts/credentials/v2
Outdated
| @@ -92,6 +92,28 @@ | |||
| } | |||
| }, | |||
|
|
|||
| "JsonSchema2023": { | |||
| "@id": | |||
| "https://www.w3.org/ns/credentials#JsonSchema2023", | |||
There was a problem hiding this comment.
@iherman vocab URL or the 2018 one?
decentralgabe
requested review from
msporny,
selfissued and
awoie
as code owners
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
There was a problem hiding this comment.
+1 to adding CredentialSchema2023 to the base context, though it probably needs a rename to bring it in line with the "we only talk about verifiable credentials" here direction of the WG. I'm a bit concerned about JsonSchema2023 and the potential conflict with PRs like #1182. I don't think there's anything wrong with JsonSchema2023, but others in the WG might take issue with it.
Approving, while noting concerns with future changes.
vocab/credentials/vocab.csv
Outdated
| JsonSchema2023,rdfs:Class,,,,,,,A type of validator that can be used to syntactically validate JSON documents using the JSON Schema language. | ||
| CredentialSchema2023,rdfs:Class,,,,,,,A type of validator that can be used to syntactically validate JSON documents packaged as Verifiable Credentials using the JSON Schema language. |
There was a problem hiding this comment.
The distinction here doesn't make sense if the base media type is application/vc+ld+json and given the direction of PRs like #1182. I'm just flagging this as a concern as this might need to be changed in the future.
There was a problem hiding this comment.
good flag. simply put, the spec operates on portions of a VC which it always treats as JSON, despite the outer wrapper being vc+ld+json. If there's a way I can make this more clear I'm open to suggestions.
There was a problem hiding this comment.
My suggestion is to just define ONE type for the schema validator you're defining in the JSON Schema 2023 specification. My suggestion is we just define "JsonSchema2023" in the v2 vocabulary, and any properties used therein, and call it a day.
If it would be helpful, I could just rework the PR to do that, as I'm pretty sure I know what you're trying to accomplish in this PR. I don't want to make those changes to this PR w/o your permission, though. So, let me know if you want me to clean up this PR and I'll be happy to do so.
There was a problem hiding this comment.
I have no problem with you making these changes
There was a problem hiding this comment.
I'm still a +1 for this PR, but caught a few things as I was going to merge it down to main. Just need @decentralgabe to confirm that he's ok w/ the modifications and I can take it from there.
Co-authored-by: Manu Sporny <msporny@digitalbazaar.com>
There was a problem hiding this comment.
Approving noting that one of the schema types might need to be removed in the future: w3c/vc-json-schema#172 (but we have the latitude to do that during CR based on the at risk issue markers).
|
Normative, multiple reviews, changes requested and made, no objections, merging. |
fix #1134