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.

Sign up for GitHub

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

Rework the data model to support statusPurpose field. #24

Merged
merged 4 commits into from
Apr 4, 2022
Merged

Rework the data model to support statusPurpose field. #24

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df4b864
Update status list 2021 data model.
msporny Mar 12, 2022
82c5c53
Update JSON-LD Context to match latest specification.
msporny Mar 12, 2022
fb40f90
Add "Other Links" section to respec.
msporny Mar 12, 2022
01a681d
Switch to StatusList2021Entry and StatusList2021Credential.
msporny Apr 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 11 additions & 26 deletions contexts/v1.jsonld
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"@context": {
"@protected": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if just include an @vocab would be a better move here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why do you think that, @OR13? If we do that, then everything under StatusList2021 becomes a part of the status list vocabulary. Seems like a dangerous thing to just blanket map any term to the status list vocabulary (this is why @vocab, in general, is a bad idea and shouldn't be used).


"StatusList2021Credential": {
"StatusListCredential2021": {
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be changed -- this is a type of Credential not a Credential2021, so it should end in just Credential.

Suggested change
"StatusListCredential2021": {
"StatusList2021Credential": {

Copy link
Member Author

@msporny msporny Mar 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, considered that. It's strange... can't think of anywhere else that we do this (date stamp in middle of type name). It creates this new rule that people have to remember when using date stamped values... and it's not clear when to use one vs. the other. I'd rather we stick with one rule (dates go at the end) rather than (dates sometimes go in the middle -- here's yet another complex VC thing you have to remember to determine when to do what).

Copy link
Contributor

Choose a reason for hiding this comment

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

The other rule we have always had is that Credential is last. This would break that rule.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I don't think that there's a "new rule" here with date stamps. I think these have already been the rules:

  1. If you want to date stamp a thing, X, with a year, YEAR, do XYEAR.
  2. If you want a new type of Credential to hold / talk about X it should be XCredential. This means XYEARCredential for date stamped X's.

I don't think we're date-stamping the credential, we're date-stamping the status list. Then, as part of how this system works, we also need a credential to put the status list in. That's what the credential is ... a credential that holds a StatusList2021. It's not a credential ... from 2021 (or something?) that is holding an unstamped status list. I'm not sure what the date stamp means if it's put at the end here.

I think it's better that the rules be that the date follows the thing it applies to -- which is the status list definition. The fact that there's a credential to go along with a StatusList2021 is orthogonal, IMO.

Copy link
Member Author

Choose a reason for hiding this comment

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

One thing I considered was just dropping the 2021 and just making it "StatusListCredential". Thoughts?

Copy link

Choose a reason for hiding this comment

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

Interesting. I get you. Seems like StatusList2021Credential agrees the "most" with naming conventions I suppose.
Other options like putting a /vYYYY/ in the path would cause issues with resolution down the line.
Seems weird, but I agree that this particular credential type is far from settled.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like StatusList2021Credential agrees the "most" with naming conventions I suppose.

I count that as 2 for, 1 undecided... would be good to get some more opinions on this... /cc @OR13 @mprorock @mkhraisha @tplooker @mavarley @TallTed @peacekeeper

Copy link
Member

Choose a reason for hiding this comment

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

StatusList2021Credential being the Credential that holds the 2021 StatusList which is made up of some number of StatusList2021Entry Entries seems to make the most sense. I think it will need some clear description of how it was arrived at, something like my first sentence, to carry forward clearly into the 2022 or 2025 or 2057 editions...

Copy link

Choose a reason for hiding this comment

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

Just so there is more consensus, XYearCredential seems fine to me. So StatusList2021Credential is ok. The naming convention reminds me of ECMA script standards such as ES2016 which are working fine in the javascript community.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 01a681d.

"@id":
"https://w3id.org/vc/status-list#StatusList2021Credential",
"@context": {
Expand All @@ -16,54 +16,39 @@
}
},

"RevocationList2021": {
"StatusList2021": {
"@id":
"https://w3id.org/vc/status-list#RevocationList2021",
"https://w3id.org/vc/status-list#StatusList2021",
"@context": {
"@protected": true,

"id": "@id",
"type": "@type",

"statusPurpose":
"https://w3id.org/vc/status-list#statusPurpose",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this is better than statusType ... but maybe. Still considering.

Copy link
Member Author

@msporny msporny Mar 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'm concerned about adding too many xType fields... people are going to get confused between that and the regular type field. Seems like there's good alignment with proofPurpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think this is probably a good approach.

Copy link

Choose a reason for hiding this comment

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

+100 @msporny

Copy link

@aljones15 aljones15 Mar 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Also agreeing here: statusPurpose is a better name than statusType which creates ambiguity with numerous other uses of the term type.

"encodedList": "https://w3id.org/vc/status-list#encodedList"
}
},

"RevocationList2021Status": {
"StatusListEntry2021": {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this should be StatusList2021Entry, i.e., everything should be StatusList2021X.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same comment as https://github.com/w3c-ccg/vc-status-list-2021/pull/24/files#r825341914.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would be good to get some more opinions on this as well. /cc @OR13 @mprorock @mkhraisha @tplooker @mavarley @TallTed @peacekeeper

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 01a681d.

"@id":
"https://w3id.org/vc/status-list#RevocationList2021Status",
"https://w3id.org/vc/status-list#StatusListEntry2021",
"@context": {
"@protected": true,

"id": "@id",
"type": "@type",

"statusListCredential": {
"@id":
"https://w3id.org/vc/status-list#statusListCredential",
"@type": "@id"
},
"statusPurpose":
"https://w3id.org/vc/status-list#statusPurpose",
"statusListIndex":
"https://w3id.org/vc/status-list#statusListIndex"
}
},

"SuspensionList2021Status": {
"@id":
"https://w3id.org/vc/status-list#SuspensionList2021Status",
"@context": {
"@protected": true,

"id": "@id",
"type": "@type",

"https://w3id.org/vc/status-list#statusListIndex",
"statusListCredential": {
"@id":
"https://w3id.org/vc/status-list#statusListCredential",
"@type": "@id"
},
"statusListIndex":
"https://w3id.org/vc/status-list#statusListIndex"
}
}
}
}
Expand Down
Loading