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

chore: update title and urls to contexts #69

Merged
merged 10 commits into from
Aug 30, 2023

Conversation

mprorock
Copy link
Contributor

@mprorock mprorock commented Jun 12, 2023

This PR will require updates to w3c ns per this comment

Versioning is also indicated in this PR as 2.0 since it corresponds to core data model 2.0 and version is indicated in the path to context: https://www.w3.org/ns/status-list/v2 - this also brings versioning of this doc in line with other recs and with the guidebook linked by current w3c materials


Preview | Diff

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.

We would need to update the V2 context as well, since it bundles terms

@mprorock
Copy link
Contributor Author

We would need to update the V2 context as well, since it bundles terms

yes, if we accept this PR there will need to be issues created and then followed on to ensure completeness

Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

Love these changes - thank you.

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.

excellent!

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
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.

Given the v2 context defines these terms as @protected including a second context that defines them will ALWAYS cause JSON-LD processing errors.

removing extra context in example

Co-authored-by: Orie Steele <[email protected]>
@mprorock
Copy link
Contributor Author

Given the v2 context defines these terms as @protected including a second context that defines them will ALWAYS cause JSON-LD processing errors.

excellent catch. thank you.

@mprorock mprorock requested a review from OR13 June 13, 2023 17:18
@mprorock mprorock mentioned this pull request Jun 13, 2023
@mprorock
Copy link
Contributor Author

We would need to update the V2 context as well, since it bundles terms

#70 opened to track this

@iherman
Copy link
Member

iherman commented Jun 14, 2023

The issue was discussed in a meeting on 2023-06-14

  • no resolutions were taken
View the transcript

1.11. chore: update title and urls to contexts (pr vc-status-list-2021#69)

See github pull request vc-status-list-2021#69.

Manu Sporny: this modifies the context and data types and things.
… i suggest implementers should review, this is a big set of breaking changes.
… I plan to request changes, and make sure that this is something we want to do.
… it impacts JSON ONLY processing in a negative way.
… it looks simple but its related to RDF data processing.

Orie Steele: There are several open PRs on vc-jwt, I will merge anything with positive reviews after 1 week.

Kristina Yasuda: lets do issues.

@msporny
Copy link
Member

msporny commented Jun 18, 2023

Given the v2 context defines these terms as @protected including a second context that defines them will ALWAYS cause JSON-LD processing errors.

excellent catch. thank you.

This is not true. Re-defining terms to the exact same protected value does not throw errors.

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.

Removing any sort of versioning from the type will make it such that it will not be possible to determine a v2 status list from a v3 status list. That seems like an issue, are the reviewers of this PR aware of that fact?

In other words, we are removing versioning entirely from status list. That seems like a really bad idea, because if we make a mistake w/ this version (which we almost certainly will), it will make it very difficult to fix in the future without re-introducing a version.

-1 to removing versioning entirely from status list types. I would approve a PR that would re-introduce a version of some kind into the status list type.

@mprorock
Copy link
Contributor Author

Removing any sort of versioning from the type will make it such that it will not be possible to determine a v2 status list from a v3 status list. That seems like an issue, are the reviewers of this PR aware of that fact?

In other words, we are removing versioning entirely from status list. That seems like a really bad idea, because if we make a mistake w/ this version (which we almost certainly will), it will make it very difficult to fix in the future without re-introducing a version.

-1 to removing versioning entirely from status list types. I would approve a PR that would re-introduce a version of some kind into the status list type.

isn't the version apparent in the context? e.g. https://www.w3.org/ns/credentials/v2 happy to add an explicit version number if you think that is better?

@dlongley
Copy link
Contributor

I can live without a version in the type names. The names are probably specific enough to avoid conflict with whatever people are modeling so that they don't have to do anything special to avoid those conflicts.

We should probably have any future revision use new names that themselves carry a bit more semantic meaning than just "status list" anyway. If we wanted to do something about that now, we could do perhaps: "BitStringStatusList" and "BitStringStatusListEntry" to reflect one of the core features of how this particular status list works.

@iherman
Copy link
Member

iherman commented Jul 12, 2023

The issue was discussed in a meeting on 2023-07-12

  • no resolutions were taken
View the transcript

4.5. chore: update title and urls to contexts (pr vc-status-list-2021#69)

See github pull request vc-status-list-2021#69.

Manu Sporny: there are big changes being suggested.
… change name, change URLs, remove versioning, etc...
… probably needs the wg to decide.
… one concern is that when we added multiple status to status list, that it will be a breaking change.
… we would need to bump the version to avoid breaking change.
… we are discussing what the normative vocab url should be.
… the other updates is to vc-specs dir.

@msporny
Copy link
Member

msporny commented Jul 17, 2023

isn't the version apparent in the context? e.g. https://www.w3.org/ns/credentials/v2 happy to add an explicit version number if you think that is better?

No, it's not, because that context (if this change was accepted) would just pull in a type called "StatusList" -- no version... and "StatusListEntry" -- no version. IOW, if we were looking to make a breaking change in the future, such as we just did for the "multibit array" stuff, we wouldn't be able to do it w/o introducing yet another type (which would either be versioned) or a new property (which would be hacky, because the only reason we're introducing it would be to work around the fact that we messed it up in this revision).

There are cases where we do this today, like: "DataIntegrityProof", which gets its versioning from "cryptosuite" and which we've had years of implementation experience on. We could make the same argument for "JsonWebKey" and "publicKeyJwk" because there is a history of backwards compat there going back almost a decade.

This spec is relatively new... and thus we don't know if we're getting it right... so we should version it until we're absolutely sure we're not going to make a change to it in the future.

@iherman
Copy link
Member

iherman commented Jul 20, 2023

The issue was discussed in a meeting on 2023-07-18

  • no resolutions were taken
View the transcript

1.1. chore: update title and urls to contexts (pr vc-status-list-2021#69)

See github pull request vc-status-list-2021#69.

Brent Zundel: What is this PR trying to do, what are next steps?

Michael Prorock: The goal here was to get stuff pointing at the v2 stuff for the most part, some items were still pointing at other items.
… Because we now have status list available under v2 of VCs, do we need to explicitly call out things by year, for example StatusList2021Entry vs. StatusListEntry. I believe we saw call out for explicit version, a little indifferent, as long as you're pointing to v2, great, also totally fine in status list explicit property v2.0, goes along w/ v2.0 VCs, fine, open to thoughts here.

Orie Steele: +1 to not adding a "version prop".

Dave Longley: can live without a version in type names - would prefer versioning other than via properties - does not think that the version is sufficient in cases of future context changes.
… years makes sense for things that have a shelf life, less sense in things that don't, perhaps we use a differentiating feature or otherwise in the type name to differentiate.

Manu Sporny: not broadly implemented and deployed yet, so we have some flexibility.
… uncomfortable removing versioning especially with breaking changes. -1 to not having some version specifically for this reason.
… options include specific naming of status list such as 'BitStringStatusList' or something very specific, or include a version on the type, context v2 is not enough to determine type.
… very uneasy with taking all versioning off of this.
… have gotten feedback from one implementer that they don't want to continue to use 2021 for current version.
… title of the spec changing to 2.0 when there wasn't a version 2.0 seems odd.

Orie Steele: feels like using @type for versioning is the same thing as "sniffing json members" for version info...
Dave Longley: Orie: the type of a thing largely defines what something is -- so i think if we're going to change what a thing is (different properties, features, core meaning, so on), it makes sense to signal that through type.
Orie Steele: Yeah, makes me wonder why https://schema.org/Organization is not https://schema.org/Organization2023.
Dave Longley: Orie: i wouldn't expect Organization to have significant breaking changes over time, only additive.
and Organization is very generic ... so matches its name... "StatusList" is a pretty specific BitString-related status list thing.
not a generic "StatusList".

Michael Prorock: I'm fine w/ a property based versioning approach, versioning in the name itself feels odd to me, because then you're switching on strings, never really a right answer. Totally fine w/ keeping some versioning, so what Manu and Dave are bringing up is fine... where is that done, in a property, name of type, somewhere else?

Ted Thibodeau Jr.: versioning good, versioning with years not so good, versions in name of context file is problematic as it disappears once it resolves through the context, putting the version on the type means that we should build range and domain attribute lists for these types and then those attributes need to update with changes, putting versioning on attributes itself might be worthwhile if we are changing the def of an attr.
… nothing is simple and when we see interdependencies that complexity around versioning gets more complex.

Orie Steele: +1 TallTed.. versioning is lost when context is applied... unless its on the type.... it implies heavier RDF work.
Manu Sporny: +1 to TallTed.

Dave Longley: what we see in the future will determine best path - statuslist is very generic - we could keep that, or we could get more specific around naming based on how this status list operates.
… feel like we are running into trouble because the name we chose is so general.
… if we are just adding backwards compat changes in the future then this is fine, but that might not be the case.

Michael Prorock: I kinda like where Dave is going with this, I think he's on to something and that's already caused some open PRs over StatusList because the name is general. I'm totally fine if we settle on a name that is specific and reflective that doesn't have a name/etc. BitsStringstatusList might be a good way to do this, it's not perfect, but at least it narrows things down. fine with that as a path forward.

Orie Steele: Why didn't we version VerifiableCredential and VerifiablePresentation ?
Dave Longley: Orie: they are generic enough and you can add lots of things to them without issue.
So same argument as Organization... and as StatusList... now...
Dave Longley: Orie: yes, except this "bit string status list" thing we have isn't a generic "status list".
we could create a base StatusList type that has nothing on it ... and then extend that with a more specific type with more requirements... or just not bother with a base type like that here.

Brent Zundel: next step? proposed path forward of getting more specific with naming? any opposition?

Manu Sporny: would feel differently if we were 5yrs in and things were very stable.
… raising concerns since this is very fresh and new, would prefer a version in the type as the easiest path to prevent collisions between versions.
… need a mechanism to identify expected properties, and then we expect only backwards compat changes.
… better than nothing if we accept very specific naming.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@mprorock
Copy link
Contributor Author

@msporny do the suggestions to adjust to 'BitStringStatusList' that i have added in a few places match a direction that will work for you? If so, I can update the rest of the PR

@msporny
Copy link
Member

msporny commented Jul 27, 2023

@msporny do the suggestions to adjust to 'BitStringStatusList' that i have added in a few places match a direction that will work for you? If so, I can update the rest of the PR

Yes, please update the name. I'd be fine w/ "TbdSpecificNameStatusList", with an issue marker saying that we're trying to decide on a specific name/names for the types, if you want to get rid of the date for now and we can bikeshed the name elsewhere.

One thing that we might want to do is create a type for a "simple bitfield" and a type for a "packed bitfield". Upside is that we simplify the logic on what properties are / are not available for each type. Downside is that we don't have a unified way of doing status lists (but, we won't in time anyway -- people are already deploying blockchain status lists, IPFS status lists, etc.).

I'm leaning towards the "two status list types" at present to provide design flexibility as we move forward... I don't feel good about making this sort of breaking change right as we're entering feature freeze. It's where we have consensus, so trying to figure out a way to help the spec survive CR.

@iherman
Copy link
Member

iherman commented Jul 27, 2023

I do not know whether this is the right time and place, or whether we want to raise a separate issue.

This document yields a number of terms that must be incorporated into the credentials vocabulary at some point. I hope I am not mistaken with this list:

  • Classes:
    • StatusListEntry
    • StatusListCredential
    • StatusList
  • Properties:
    • statusPurpose
      • range: xsd:string
      • domain: StatusListEntry or StatusList
    • statusListIndex
      • range: xsd:string
      • domain: StatusListEntry
    • statusListCredential
      • range: StatusListCredential
      • domain: StatusListEntry
    • encodedList
      • range: xsd:string
      • domain: StatusList

I presume that these terms will be formally defined by this specification (which is perfectly fine from my point of view). However, to anticipate this (see w3c/vc-data-model#1209) there should be clear anchors that provide the referencable URLs for the vocabulary entry into this document. At the moment there is a URL forStatusListEntry and StatusListCredentials, but all the others are missing. It may be as simple as providing an @id attribute on the table cell elements for the properties, but this will miss the reference for StatusList (which is "hidden" in the table row for credentialSubject.type).

Again, if you prefer to merge this PR as is I can raise a separate issue.


Discussion has been moved to #81

@msporny
Copy link
Member

msporny commented Jul 27, 2023

This document yields a number of terms that must be incorporated into the credentials vocabulary at some point. I hope I am not mistaken with this list:

We should probably have a different vocabulary for credential status. It's not required, but it might be the best way to structure things (separation of vocabulary purposes... where the credential vocabulary is for core VCDM things... and extensions get their own vocabularies)? Note that we will need to produce a separate JSON-LD context for status list because of people wanting to pull it into V1 VCs or other software systems (possibly).

This is definitely not the PR to have this discussion, though, so we should raise a separate issue, IMHO.

@iherman iherman mentioned this pull request Jul 28, 2023
@iherman
Copy link
Member

iherman commented Jul 28, 2023

@msporny I have created an issue (#81) copying my original question.

@mprorock mprorock requested a review from msporny August 4, 2023 22:07
@msporny
Copy link
Member

msporny commented Aug 10, 2023

@mprorock please fix conflicts so we can merge.

@msporny msporny reopened this Aug 21, 2023
@mprorock
Copy link
Contributor Author

@msporny suggest we squash - i can't figure out why this won't allow a rebase in the UI

@msporny msporny merged commit 06e5483 into w3c:main Aug 30, 2023
1 check passed
@msporny
Copy link
Member

msporny commented Aug 30, 2023

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

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