-
Notifications
You must be signed in to change notification settings - Fork 110
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
IssuanceDate and ExpirationDate #964
Conversation
index.html
Outdated
"holder": {"@id": "cred:holder", "@type": "@id"}, | ||
"issued": {"@id": "cred:issued", "@type": "xsd:dateTime"}, | ||
"issuer": {"@id": "cred:issuer", "@type": "@id"}, | ||
"issuanceDate": {"@id": "cred:issuanceDate", "@type": "xsd:dateTime"}, | ||
"validFrom": {"@id": "cred:validFrom", "@type": "xsd:dateTime"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that validUntil
and validFrom
are already present in the context, at the end -- so these are duplicates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, should be resolved now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was just about to do this 😄
only question - do the existing context's need to be maintained for backwards compatibility?
I believe so, this PR only affects v2. |
@@ -17,9 +17,9 @@ property,credentialSchema,,,credential schema,,cred:VerifiableCredential,cred:Cr | |||
property,credentialStatus,,,credential status,,cred:VerifiableCredential,cred:CredentialStatus,,The value of the `credentialStatus` property MUST be an instance of Credential Status. | |||
property,credentialSubject,,,credential subject,,cred:VerifiableCredential,,,An entity about which claims are made. | |||
property,evidence,,,evidence,,cred:VerifiableCredential,cred:CredentialEvidence,,The value of the `evidence` property MUST be one or more Credential Evidence instances. | |||
property,expirationDate,,,expiration date,,cred:VerifiableCredential,xsd:dateTime,,The value of the `expirationDate` property MUST be a string value of an ISO8601 combined date and time string representing the date and time the credential ceases to be valid. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that everything below here is going to conflict with https://github.com/w3c/vc-data-model/pull/968/files. You are either going to want to rebase on top of PR #968 or have @iherman rebase on top of your PR. Unfortunately, both of your PRs are making changes to a shared set of files. :)
The issue was discussed in a meeting on 2022-11-02
View the transcript1.1. IssuanceDate and ExpirationDate (pr vc-data-model#964)See github pull request vc-data-model#964. Manu Sporny: simultaneously ivan raised a PR to update vocabularies. See github pull request vc-data-model#968. Manu Sporny: and they conflict with one another. Ivan Herman: my PR does two things. one yaml is now the language and the other thing it also includes the changes of property names. Manu Sporny: that is great because we have more people doing PRs. |
@mkhraisha I was looking at this in view of the conflicts with #968. The conflicts are exclusively on the vocabulary part, ie, The problem is that #968 pretty much changes the way the ttl/jsonld/html files are generated. In the older version the starting point was the CSV file, whereas in the new version of the vocabulary generation the starting point is a YAML file. Also, I am not sure whether you changed the ttl/jsonld/html files by hand or you ran the generation scripts; if the former, that is a problem by itself, those files are generated. #968 also takes care of the core issue of your PR, namely the new properties are introduced. Technically, that PR does not remove the old properties but, instead, deprecates them; because the vocabulary is already deployed, it is not a good idea to simply remove them from the vocabulary, thereby creating dangling links on the linked data world. To move forward: would it be possible for you to roll back all the changes you have made on If that is too complicated, then another approach is that this PR is merged, and I can then either rebase my changes on (the new) Whichever is simpler for you (unless @msporny comes up with a better idea). |
@iherman probably just easiest to close this PR and if the issue still persists after your PR is merged then I can recreate it. This was not a lot of work for me. |
Closing this PR in favor of #968. Once we merge #968, we can come back to this one. |
PR tackles #913
Updated Diagrams, Explainer, V2 context csv and used the npm generate script, removed the index.html deprecation warnings.
Preview | Diff