Skip to content

Remove termsOfUse from v2 context #1093

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

Closed
wants to merge 2 commits into from
Closed

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Apr 18, 2023

Addresses #1090


Preview | Diff

Copy link
Member

@msporny msporny left a comment

Choose a reason for hiding this comment

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

-1 for the following reasons:

  • The PR title is not accurate, it removes refreshService as well as termsOfUse.
  • This term has already been used as an extension point in the VC Specifications Directory.
  • It only impacts JSON-LD implementations (and the cost is a handful of bytes for a context that is permanently cached and never downloaded).
  • There is active interest in this extension point (as evidenced by TRAIN work in the EU).

@iherman
Copy link
Member

iherman commented Apr 19, 2023

-1

This seems to be a rushed PR on an unfinished discussion yesterday, namely whether references to an "experimental" (whatever name we will use) term should be removed from the @context file or not. In my view, unless the term is fully removed from all our documents, this should not be done.

As I said on the call, as long as the term has a URL in our vocabulary (https://www.w3.org/2018/credentials#termsOfUse in this case) the term should remain in the @context file. And we did agree on the call (although there were no formal resolution) that if we keep a term in the VCDM (regardless whether it is a "experimental" or a bona fide standard term) then we also keep it in the vocabulary with the right URL.

The @context file does not define anything whatsoever. The term in question is defined in the VCDM specification and the VCDM vocabulary document. In the sense of the specification, the @context file is merely a helpful tool for JSON-LD implementers to make it sure that when they use the JSON key termOfUse, they indeed use the one the WG has agreed upon. Fundamentally, that is al it does. If the item is removed from the @context, by virtue of the @vocab that we introduced in @context, the term will instead be turned into https://www.w3.org/ns/credentials/issuer-dependent#termOfuse, which is not what we would want.

@OR13
Copy link
Contributor Author

OR13 commented Apr 19, 2023

Fundamentally, that is al it does. If the item is removed from the @context, by virtue of the @vocab that we introduced in @context, the term will instead be turned into https://www.w3.org/ns/credentials/issuer-dependent#termOfuse, which is not what we would want.

This is what I would expect for a term that the working group has not agreed to use, but for which an issuer has found value.

I stated this at the end of the call.

If the working group has agreed to this term, as a normative requirement, it should not be reserved, and it should be defined in the TR AND in the JSON-LD context.

We should avoid bundling non normative stuff into the JSON-LD context, since they are cached, they can be split by normative and non normative easily, and then implementers can more easily signal their intention to use experimental extensions.

@dlongley
Copy link
Contributor

dlongley commented Apr 19, 2023

@OR13,

Fundamentally, that is al it does. If the item is removed from the @context, by virtue of the @vocab that we introduced in @context, the term will instead be turned into https://www.w3.org/ns/credentials/issuer-dependent#termOfuse, which is not what we would want.

This is what I would expect for a term that the working group has not agreed to use, but for which an issuer has found value.

From my perspective, this misses the point and distinction introduced by the prospective ("reserved") terms table. The terms we put in that table are specifically called out as not being "issuer-defined", but rather prospective terms that will be defined by the WG in the future (or by a future WG) ... and therefore independent of any issuer. They map to URLs in the WG-defined vocabulary.

@David-Chadwick
Copy link
Contributor

-1 to this request

@longpd
Copy link
Contributor

longpd commented Apr 24, 2023

-1 to this request, as well. I concur with @dlongley, @iherman and @msporny. @context files are implementation tools to make easier and more clear what is being referenced as defined by the VCWG.

@TallTed
Copy link
Member

TallTed commented Apr 25, 2023

-1 from me, too.</aol>

Copy link
Contributor

@decentralgabe decentralgabe left a comment

Choose a reason for hiding this comment

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

if it were just removing termsOfUse and not also refreshService I'd approve

@msporny msporny added DO NOT MERGE PR contains something that should not be merged. discuss labels Apr 27, 2023
@brentzundel
Copy link
Member

There does not seem to be a path toward consensus that would result in merging this PR.
I am adding the pending-close label and will close in 1 week if consensus still seems unlikely.

@brentzundel brentzundel removed DO NOT MERGE PR contains something that should not be merged. discuss labels May 3, 2023
@OR13
Copy link
Contributor Author

OR13 commented May 4, 2023

I thought we had agreement, that we would be reserving this URL and term name, and not defining it or using it in the v2 spec?

@msporny
Copy link
Member

msporny commented May 13, 2023

I thought we had agreement, that we would be reserving this URL and term name, and not defining it or using it in the v2 spec?

Removing both declarations from the v2 context (what this PR does) is something that we did not have consensus on, and are unlikely to get consensus on it. It's been a week since being marked pending close. Closing.

@msporny msporny closed this May 13, 2023
@msporny msporny deleted the remove-termsOfUse-from-v2-context branch July 27, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending close Close if no objection within 7 days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants