Skip to content
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

Updated the URL-s in the vocabulary #212

Merged
merged 4 commits into from
Dec 10, 2023
Merged

Conversation

iherman
Copy link
Member

@iherman iherman commented Oct 16, 2023

This is an editorial cleanup for the URL-s in the voabulary, following the separate discussion in w3c/vc-data-model#1296 (comment) but also w3c/vc-data-model#1296 (comment).

The generated files are not on /TR, so this PR does not affect the current CR review.

@iherman iherman self-assigned this Oct 16, 2023
@iherman iherman added the CR1 This item was processed during the first Candidate Recommendation phase. label Oct 16, 2023
<mxCell style="rounded=1;whiteSpace=wrap;html=1;fontSize=16;fillColor=none;strokeWidth=2;" parent="1" vertex="1">
<mxGeometry x="555.3489294710328" y="440.29808823529413" width="163.7314231738035" height="32.536500000000004" as="geometry" />
</mxCell>
</UserObject>
<UserObject label="&lt;i&gt;expires&lt;/i&gt;" link="https://w3.org/2018/credentials/#expires" id="Uf8WLKuzS3drS_BCJ-BJ-12">
<UserObject label="&lt;i&gt;expires&lt;/i&gt;" link="https://w3id.org/security#expires" id="Uf8WLKuzS3drS_BCJ-BJ-12">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be "expiration" ... but perhaps not for this PR. The context term is "expires" but the URL uses "#expiration". It's a unicorn, but has been in use for years that way.

Suggested change
<UserObject label="&lt;i&gt;expires&lt;/i&gt;" link="https://w3id.org/security#expires" id="Uf8WLKuzS3drS_BCJ-BJ-12">
<UserObject label="&lt;i&gt;expires&lt;/i&gt;" link="https://w3id.org/security#expiration" id="Uf8WLKuzS3drS_BCJ-BJ-12">

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dlongley Ah. So the term is expires in the spec, and, indeed, the @context uses expiration in the URL. Ouch.

There seems to be two ways to change this:

  • Set the URL in the vocabulary to expiration rather than expires (the fact that the diagram uses expires is only an artifact). That means the vocabulary will be at odds with what the spec says, which is ugly.
  • Update the @context file to the right ID

The second option seems to be the correct way of doing this. Any reason why we should not?

(But yes, I believe we should leave this for a separate PR.)

Copy link
Contributor

@dlongley dlongley Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second option seems to be the correct way of doing this. Any reason why we should not?

Yes, the reason not to do it is that many things have shipped / escaped into the wild using #expiration in the URL. The contexts have been that way for a long while and we should probably just embrace it. The vocab was erroneously changed to use #expires in some recent update that no one picked up on because it is a special unicorn -- and it should be changed back to #expiration, IMO. It's not pretty, but it is the way it's been done for a long time now and I don't think it buys us much to try and change to #expires -- with any number of possible issues coming up in the ecosystem if we do.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am holding my nose but... I will do it tomorrow. It is more than what you put there as a change request: the change must also be made in the svg file and, more importantly, in the vocabulary.yml file as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dlongley @msporny I have made the expires -> expiration change in 8afd6b9.

I was wondering whether we should put a small text as a comment into the vocabulary file explaining why, if one dereferences the definition link, one gets to the term expires in the VCDM spec. Because it seems to have a historical background, I would prefer one of you two to propose a text. Alternatively, we may remain silent...

(If you decide that a text is better, you can directly edit the yml file by adding a comment: field to the definition of expiration)

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An attempt to clarify through some language tweaks...

vocab/security/vocabulary.yml Outdated Show resolved Hide resolved
@msporny
Copy link
Member

msporny commented Dec 10, 2023

Editorial, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit 14e0254 into main Dec 10, 2023
2 checks passed
@msporny msporny deleted the URL-cleanup-in-the-vocabulary branch December 10, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR1 This item was processed during the first Candidate Recommendation phase.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants