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

Add first pass at reserved properties table #1097

Merged
merged 5 commits into from
May 4, 2023
Merged

Conversation

msporny
Copy link
Member

@msporny msporny commented Apr 22, 2023

This adds a first pass at the reserved properties table (as suggested in PR https://github.com/w3c/vc-data-model/pull/1082/files#r1170186655 by @mprorock).


Preview | Diff

@David-Chadwick
Copy link
Contributor

The definition of ToU in the table conflicts with the current ToU definition in the main body of the specification.

Is it proposed that the main section on ToU will be deleted from v2 DM or left in with along with its addition to the reserved properties?

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.

LGTM for a starting point

</td>
</tr>
<tr>
<td>`evidence`</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

need to reserve this term URL here, as well:

https://www.w3.org/2018/credentials#evidence

Copy link
Member

Choose a reason for hiding this comment

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

Correct. I plan to make a revision of the vocabulary document, as well as the yml2vocab tool (so that experimental terms are presented separately) if and when these PR are merged.

index.html Outdated
</td>
</tr>
<tr>
<td>`renderMethod`</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

need to reserve new URL for this under W3C or W3ID.

Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

i need time to think about this (i was travelling the last couple of weeks)

index.html Outdated
<tbody>
<tr>
<td>`confirmationMethod` / `confidenceMethod`</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll accept this PR as-is since there's an issue in here. Just expressing my preference:

Suggested change
<td>`confirmationMethod` / `confidenceMethod`</td>
<td>`confidenceMethod`</td>

Copy link
Contributor

Choose a reason for hiding this comment

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

We haven't agreed on either yet. I'd prefer to keep both until we resolved the naming discussion.

Base automatically changed from msporny-reserved-properties to main April 24, 2023 23:54
@awoie
Copy link
Contributor

awoie commented Apr 25, 2023

i need time to think about this (i was travelling the last couple of weeks)

@msporny Would we still add sections to the VC Directory for "reserved" properties?

Copy link
Contributor

@David-Chadwick David-Chadwick left a comment

Choose a reason for hiding this comment

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

I dont think it makes sense to have a property that is both defined in the V2DM and is included in the reserved properties table. They should be mutually exclusive, since a property in the V2DM is automatically reserved. Consequently this PR should strike out each of the sections in the DM that are included in the reserved properties table.

@msporny
Copy link
Member Author

msporny commented Apr 25, 2023

i need time to think about this (i was travelling the last couple of weeks)

@msporny Would we still add sections to the VC Directory for "reserved" properties?

This is my Editorial option, and opinion as a manager of the VC Specs Directory -- yes, we would add sections to the VC Specs Directory for all reserved properties. One of the reasons for the table is to establish which extension sections exist in the VC Specs Directory. Others in the group might disagree with this notion, but if we don't have this mechanism to establish sections in the VC Specs Directory, then the question becomes "Then exactly what establishes those sections?".

@awoie
Copy link
Contributor

awoie commented Apr 25, 2023

i need time to think about this (i was travelling the last couple of weeks)

@msporny Would we still add sections to the VC Directory for "reserved" properties?

This is my Editorial option, and opinion as a manager of the VC Specs Directory -- yes, we would add sections to the VC Specs Directory for all reserved properties. One of the reasons for the table is to establish which extension sections exist in the VC Specs Directory. Others in the group might disagree with this notion, but if we don't have this mechanism to establish sections in the VC Specs Directory, then the question becomes "Then exactly what establishes those sections?".

I cannot approve until we have agreed to add sections to the VC directory for reserved properties. Otherwise, I want to discuss why certain reserved properties have to be on the list.

@msporny
Copy link
Member Author

msporny commented Apr 25, 2023

@David-Chadwick wrote:

I dont think it makes sense to have a property that is both defined in the V2DM and is included in the reserved properties table. They should be mutually exclusive, since a property in the V2DM is automatically reserved. Consequently this PR should strike out each of the sections in the DM that are included in the reserved properties table.

Yes, agreed. There are a few paths that the WG could consider wrt. the issue you raise, @David-Chadwick.

  1. All properties listed in the table are included in the specification, but with "at risk" markers noting that if an implementation of the property extension point, with a specification, test suite, and at least two demonstrably interoperable implementations do not materialize (in the VC Specs Directory) by the end of the Candidate Recommendation phase, that the section will be removed from the specification.
  2. Any property listed in the table is removed from the specification. So, evidence and termsOfUse would be removed from the specification. To get back into the specification, implementers will need to demonstrate at least two demonstrably interoperable implementations by the end of the Candidate Recommendation phase (in a few months).
  3. We keep sections for properties that were defined before in the specification (but downgrade them to non-normative), add descriptive sections for reserved properties (again, non-normative) and 1) note that we are looking for implementation feedback through CR and might make the sections normative if they get enough implementations by the end of the CR period OR 2) if they don't get enough implementations by the end of CR, remove all text in the section, say the feature didn't get enough implementation support by the v2.0 deadline, and point to the VC Specs Directory for more information on the feature.

My personal choices would be for (in order of preference) 1, 2, and 3.

@msporny
Copy link
Member Author

msporny commented Apr 25, 2023

@awoie wrote:

I cannot approve until we have agreed to add sections to the VC directory for reserved properties. Otherwise, I want to discuss why certain reserved properties have to be on the list.

I expect that will be a part of the discussion. I had presumed that no one was pushing back on the concept of "if there is an extension point listed in a normative section of the VCDM, then the equivalent section will be created for that extension point in the VC Specs Directory.

Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

Prefer to cover only the existing terms that shipped in v1 and v1.1 contexts, and then follow up on "new terms" such as "render", "confirmationMethod", "presentationSchema", etc...

@iherman
Copy link
Member

iherman commented Apr 27, 2023

The issue was discussed in a meeting on 2023-04-26

  • no resolutions were taken
View the transcript

2.2. Add first pass at reserved properties table (pr vc-data-model#1097)

See github pull request vc-data-model#1097.

Manu Sporny: some new PRs were added recently, suggest we deep dive on #1097.

Brent Zundel: agree we should, 5-10 minutes (ish), as there are other PRs and work items to look at - this would be time well spent.

Manu Sporny: #1100 is potentially important for the group, but I have not had time to review yet.
… bg regarding having merged reserved properties table.
#1097 is language from Mike Prorock regarding what this table should contain.
… There are several ways to decide what this table means.
… 1. we need a mechanism for extension point definitions.
… what happens to existing sections in specs, e.g., 'Evidence', that is also now in this table as a reserved property?
… one proposal is that anything in the table is removed from the spec UNTIL it is demonstrated to the WG that there are two independent.
… implementations for a property at which time it would go back in the spec.
… 3. reserving json terms here, not vocab, what is happening here?
… one suggestion is that we are putting a term in the table and also urls for the vocab terms in here.
… e.g., evidence has an entry in the vocab (long url for credential namespace hashed evidence). we would define both here to specify it is reserved.
… it would show up in the vocab as reserved as well.

Orie Steele: I'm in favor of getting this into the place where it can be merged. it is doing too much now.
… reserving new terms, addressing previous terms not having implementations.
… we should focus on the latter "terms of use", "refresh service", "evidence".
… all we need in the table is the json member and the URL, and only for terms shipped in v1 or v1.1 in the PR.
… this should get rid of objections, which are primarily about new terms being added, "confirmation method", "presentations schema", "render Method".
… splitting gets easier to get consensus.

Brent Zundel: +1 to breaking this up into different PRs.

David Chadwick: regarding conformance test, the three already in 1.1 already have conformance tests since the are MUST fields.
… our implementation implemented both [sic] of them.
… only two tests are needed, one is that you send to a SUT, here is a cred with a type, if you accept that is proven that it works.
… second test is to send terms or evidence without a type, and implementation should reject it.
… these should be sufficient to say we have multiple independent implementations to prove these work.
… then people can test types from registry.

Manu Sporny: going with Orie's suggestion, the concrete change request is "lets just deal with the things in the spec right now", and then define URLs for that.
… I don't know if I agree, because something has to have the normative definition for the vocab as it stands today.
… we might still need description in the URL because we are going to remove from the core spec and that is supposed to be normative.
… DavidC to your point, v1.1 and 1.0 did absolutely test that, and we are raising the bar - we need to see real usage and deployment of.
… this extension point. If we don't have that it doesn't belong in the core specification.

Kristina Yasuda: +1 manu.

David Chadwick: if you are going down to the type level, you are going to cut the group up into small groups, each of which may have implemented.
… one of the types - that seems unfair, as the rest of the datamodel is for everyone.
… it is going to be much harder to get multiple implementations of any particular type.

Brent Zundel: it was made clear when drafting the charter that the expectation was any normative statements would have an implementation.
… extensions from round 1 are moving to a table of reserved properties, until and unless there are normatively defined statements for the types.
… it will be problematic if there are terms in the data model that don't point to a normatively defined type.

Dave Longley: +1 to ensure we enable innovation somehow (objectors should not be able to block it).

Brent Zundel: the bar needs to be raised for v2.

Manu Sporny: +1 to brent, if we don't do something about this we will see formal objections when we try to exit to CR.
… chairs should as if anyone would formally object if we went down this path (primarily a question for DavidC).
… I will update the PR to reflect what I heard in the call today, and my expectation is that we need to merge or figure out which set of formal objections we are going to be happy with.

Orie Steele: even keeping the table of reserved terms is suspect, if you can't use them without types, and we have no normative types... but it seems a good path forward, from where we are today.

David Chadwick: brent said he wants the types to be formally standardized, and this worries me. That will be very difficult.
… two orgs might agree and implement, and that would meet the two implementation bar.
… going back to X509, that introduce the first extension point.

Orie Steele: We have types for credentialSchema and credentialStatus, and proof, we have reserved predicates for the ones we don't have as work items.

David Chadwick: standardizing the way to do extensions is the way to go, so people can experiment and some results will be come standards.

Manu Sporny: we are not saying you have to formally standardize things at these extension points.
… only that something has to exist on the web at a URL and there have to be two interoperable implementations.
… That is the bar for anything we do in the WG.

Phillip Long: manu just cleared up my concern.
… I was worried we were eliminating opportunities to experiment and try out, as for termsOfUse for example.
… as long as the bar is set as Manu described, I'm ok with that.

Manu Sporny: yes, Phil, that's the bar that's suggested.

Brent Zundel: responding to DavidC, from my point of view we have a standardized method of extending the datamodel - JSON-LD.
… we are allowing for certain named properties to be reserved so that the experimentation can be guided somewhat, but there is nothing.
… blocking people from experimenting as they want to.
… I could come to accept Manu's bar, but fear that it is too low - we really need high standards for our standard.
… we need to be as interoperable as possible while also encouraging experimentation and growth.

Phillip Long: I don't think a bar that supports experiments around reserved properties is too low. It's actually crucial to the evolution of the standard.

Kristina Yasuda: direction we are going is splitting PR in two as discussed; still need to agree on the bar that is needed to "get back in" to the core DM.

Chris Abernethy: short discussion of fpwd for vc-jwt.

Manu Sporny: question - we said Orie was going to make his changes and wg was going to review... did anyone review?

Orie Steele: we didn't make any changes, I sent email about this to the list. We were intending to make changes and met as editors and decided we didn't need to make any changes.

@msporny
Copy link
Member Author

msporny commented Apr 27, 2023

Based on the discussion yesterday, I have removed the proposed new terms and only included properties that existed in v1.1. Per @OR13's request, I have also asserted the URLs associated with each term as normative statements.

This should address almost all of the concerns raised on the call.

The only one I'm unsure of is @David-Chadwick, to whom I noted that I'd revise the PR and then re-request his review on these three items.

The presumption with this PR is that if it goes through, each section associated with the property would be removed from the specification and an entry would be added at the end of the specification, in the "Revision History" section, noting the removal and pointing to the respective section in the v1.1 specification (so that people can find the v1.1 sections if they wanted to). An alternative would be to link to the v1.1 sections in the table. I could go either way and would like feedback from anyone seeing this comment on what they'd prefer to see in a future PR, when we remove the sections from the spec.

@OR13 OR13 requested a review from awoie April 27, 2023 20:41
@OR13
Copy link
Contributor

OR13 commented Apr 27, 2023

@awoie can you please re-review?

@brentzundel
Copy link
Member

I believe this is ready to merge

@msporny
Copy link
Member Author

msporny commented May 4, 2023

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

@msporny msporny merged commit 2019010 into main May 4, 2023
@msporny msporny deleted the msporny-reserved-table branch May 4, 2023 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants