Skip to content

Conversation

kdenhartog
Copy link
Member

@kdenhartog kdenhartog commented Aug 30, 2021

closes #729

Originally it was pointed out that only the issuer should be added to this example. However, in order to remain consistent with all examples including all the require properties I added issuanceDate as well since it's also a required property.


Preview | Diff

@kdenhartog kdenhartog added editorial Purely editorial changes to the specification. errata Erratum for a W3C Recommendation v1.1 labels Aug 30, 2021
@kdenhartog kdenhartog self-assigned this Aug 30, 2021
kdenhartog and others added 2 commits August 31, 2021 09:41
Co-authored-by: Brent Zundel <brent.zundel@gmail.com>
Co-authored-by: Brent Zundel <brent.zundel@gmail.com>
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.

Make all issuanceDate values date strings instead of datetime strings

kdenhartog and others added 3 commits September 1, 2021 10:18
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
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.

LGTM. Please squash merge so the history doesn't get a bunch of commits w/ the same commit message (there's something strange going on w/ the commits in this PR). We try to keep commit history very clean and linear (never merge commit, always either squash (like for this PR) or ideally, rebase and merge (to create a linear history).

@kdenhartog
Copy link
Member Author

kdenhartog commented Sep 8, 2021

(there's something strange going on w/ the commits in this PR)

There was a bunch of suggestions here and copy-pasted the commit message titles based on what was in the suggestion which is why they all look the same. I plan to squash them, and clean up the commit messages in the commit message body making sure that all co-authors are kept.

@iherman
Copy link
Member

iherman commented Sep 9, 2021

The issue was discussed in a meeting on 2021-09-08

  • no resolutions were taken
View the transcript

3.3. add issuer and issuanceDate to examples (pr vc-data-model#805)

See github pull request #805.

Brent Zundel: Moving on to PR 805
… Issuer and issuanceDate - our examples didn't have issuer and issuanceDate
… So this PR adds issuer and issuanceDate to every example.
… I believe in the end they were all caught.
… If you have not yet approved this pull request, I encourage you to jump in and take a look at it.

David Chadwick: I agree with it. I've also raised a separate issue. It's part of the Credential and Verifiable Credential issue.
… I think we should remove the proof property from all, and make them all Credentials. By putting a proof property in...
… We ought to remove that, because proofs are not in credentials, only in some verifiable credentials.
… There is a section totally dedicated to proofs, and that's where it should be introduced - and no problem using it in that section.
… But in other sections, it should not be there, especially when it's "proof": ...
… The reason is that originally we thought all credentials would have proof - but then with JWTs, there was a disagreement that Manu did not want it

@msporny
Copy link
Member

msporny commented Sep 15, 2021

Editorial, multiple positive reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit 6fd8403 into v1.1 Sep 15, 2021
@msporny msporny deleted the kdh/issue-729 branch September 29, 2021 23:41
@kdenhartog kdenhartog removed editorial Purely editorial changes to the specification. errata Erratum for a W3C Recommendation labels Nov 18, 2021
@kdenhartog
Copy link
Member Author

There's a linked issue with the Errata label, removing the Errata and Editorial labels from the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing "issuer" property in examples

6 participants