-
Notifications
You must be signed in to change notification settings - Fork 116
Vocabulary synced with v2 context #927
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
Note that there is a clear relationship to #913: it is still pending whether issuanceDate and expirationDate should be deprecated. Maybe it is better to hold on the deprecation of those until that issue is settled. |
The issue was discussed in a meeting on 2022-09-07
View the transcript1.4. Vocabulary synced with v2 context (pr vc-data-model#927)See github pull request vc-data-model#927. Brent Zundel: Finally, the last one remaining open is 927. Ivan updated the vocabulary to match the VC context. Ivan Herman: For 927, I have a question there that it would be good to have an answer to. Manu Sporny: It has been deprecated. Ivan Herman: After that, then we can merge. |
The issue was discussed in a meeting on 2022-09-07
View the transcript2.2.
|
Based on the meeting of 2022-09-07 I have
@msporny you could now merge this PR... |
The PR is arguably substantive, though it's just documenting deprecated things that we should have already documented. I think we'll need to wait 5 more days for other reviews to come in (to stick to our WG workmode) before merging. I have a feeling that if I merged this now, there would be no objections, but don't want to unnecessarily push things. This will be merged in time... I've already reviewed and approved. |
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.
a few minor comments, but overall LGTM
vocab/credentials/v2/mk_vocab.rb
Outdated
@@ -248,6 +253,8 @@ def to_ttl | |||
output << "\n# Class definitions" unless @classes.empty? | |||
@classes.each do |id, entry| | |||
output << "cred:#{id} a rdfs:Class;" | |||
output << %( a owl:DeprecatedClass;) if entry[:deprecated] | |||
output << %( owl:deprecated "true"^^xsd:boolean;) if entry[:deprecated] |
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 not use the simple form true
instead?
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.
Because... I forgot:-)
vocab/credentials/v2/mk_vocab.rb
Outdated
@@ -121,6 +122,8 @@ def to_jsonld | |||
'rdfs:label' => {"en" => entry[:label].to_s}, | |||
'rdfs:comment' => {"en" => entry[:comment].to_s}, | |||
} | |||
node['owl:deprecated'] = 'true' if entry[:deprecated] | |||
node['@type'] = ['rdf:Property','owl:DeprecatedClass'] if entry[:deprecated] |
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 not clear to me why deprecated classes become properties...
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.
Because yours truly is careless and made a copy/paste error...
Substantive, multiple reviews, changes requested and made, no objections, merging. |
The PR attempts to synchronize with the V2 context. The following changes have been made:
I Have added the properties which were missing from v2, namely:
holder
issued
(I presume the missing
holder
property was actually a bug in the v1 version of the vocabulary). Note that the domain ofholder
is a bit more complicated because it can either be a Verifiable Credential or a Verifiable Presentation.Some properties are not in the v2 context anymore. Because the decision is that the v2 version of the vocabulary, when "published", will reside on the same URL as v1, removing those properties from the vocabulary is not an option. What I propose is to formally "deprecate" those terms (and classes, if applicable) in the vocabulary; there is a well defined property, and corresponding classes, in the owl namespace exactly for that. I have therefore modified the underlying script and csv file to include the possibility for deprecation, and I have set as deprecated:
I was not sure whether the
JsonSchemaValidator2018
andManualRefreshService2018
should also be deprecated or not; please advise.I have also made some editorial changes on the HTML output, but those are secondary (but it explains some of the change reports in the diff file below).
For the ease of reviewing, see:
(Reviewers: any proposed change on the term descriptions, label, etc, should be made on the CSV file. Everything else is generated...)