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

Credentials and Verifiable Credentials #808

Merged
merged 6 commits into from Sep 15, 2021
Merged

Credentials and Verifiable Credentials #808

merged 6 commits into from Sep 15, 2021

Conversation

David-Chadwick
Copy link
Contributor

@David-Chadwick David-Chadwick commented Aug 30, 2021

This PR is text for issue #798


Preview | Diff

index.html Outdated
Comment on lines 446 to 448
<a>Holders</a> assemble collections of <a>verifiable credentials</a> from
different <a>issuers</a> into a single artifact, a
<a>verifiable presentation</a>.
<a>presentation</a>.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<a>Holders</a> assemble collections of <a>verifiable credentials</a> from
different <a>issuers</a> into a single artifact, a
<a>verifiable presentation</a>.
<a>presentation</a>.
<a>Holders</a> assemble collections of <a>credentials</a> and/or
<a>verifiable credentials</a> from different <a>issuers</a> into a single
artifact, a <a>presentation</a>.

Copy link
Member

Choose a reason for hiding this comment

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

@David-Chadwick -- Since you explicitly noted agreement elsewhere, and not here, I have to ask whether you agree or disagree with this edit, which is not properly highlighted by Github. To be clear, I changed --

collections of <a>verifiable credentials</a> from different

-- to --

collections of <a>credentials</a> and/or <a>verifiable credentials</a> from different

Copy link
Member

Choose a reason for hiding this comment

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

@David-Chadwick -- This change still appears not to have been made. Do you disagree with it?

Copy link
Member

Choose a reason for hiding this comment

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

@David-Chadwick -- Still waiting on this one. Other changes (from me and @msporny) have been applied during that wait.

What's wrong with this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe that holders assemble credentials into presentations, but only verifiable credentials, as per my original text

Copy link
Member

Choose a reason for hiding this comment

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

I do still await commitment of this suggestion, as well as @dlongley's suggestion below.

For future reference, silent disagreement with a change request, in apparent hopes it will disappear when the rest of a PR is merged, is rather passive-aggressive, and is not conducive to civil discourse. If a PR author disagrees with a change request to their PR, it is incumbent upon them to respond to that request, not by silently resolving the conversation (without committing the request), nor by silently ignoring it; the PR author is expected to engage in dialog with the change requester (and/or others in the relevant community, here being the VCWG and CCG) toward finding a mutually (or at least group) agreeable revision.

(@burnburn @brentzundel @msporny @dlongley -- As chairs and editors, please correct me if I'm wrong in my assertions above.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please read my earlier post in which I categorically stated "Ok I will accept all the proposed changes and update the PR." So your comment above is uncalled for since I neither ignored your changes nor suggested alternative ones. I have stated that I have accepted them. So please be patient and wait for the updated PR.

Copy link
Member

Choose a reason for hiding this comment

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

Please read my earlier post in which I categorically stated "Ok I will accept all the proposed changes and update the PR." So your comment above is uncalled for since I neither ignored your changes nor suggested alternative ones. I have stated that I have accepted them. So please be patient and wait for the updated PR.

I waited 8 days for response to one change. Which response I only received after directly asking for such multiple times and enlisting others to the same. Unlike the other change requests which were committed without such delay.

I look forward to your updated PR, which should only require that you click a couple of "Commit suggestion" or "Add suggestion to batch" buttons.

Copy link
Member

Choose a reason for hiding this comment

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

This does not appear to have been applied.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried to accept this, but have got into a tangle that I don't know how to untangle. Sorry!

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@kdenhartog kdenhartog added editorial Purely editorial changes to the specification. errata Erratum for a W3C Recommendation v1.1 merge after 14 days labels Aug 31, 2021
apply suggestions approved by @David-Chadwick

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 as long as @TallTed and my changes are taken into account.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@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.1. Credentials and Verifiable Credentials (pr vc-data-model#808)

See github pull request #808.

Brent Zundel: Credentials and Verifiable Credentials - David's raised this, it's responding to an issue.
… I think there's only one question here. David, Ted made a suggestion, actually several, and you approved all of them but one. We weren't sure if that was an oversight or because you didn't agree with the change.
… If you agree, we can commit the suggestion with Ted's approval and get smooth sailing on this PR

Brent Zundel: https://github.com/w3c/vc-data-model/pull/808/files#r698749046

David Chadwick: OK, I thought I had agreed with all these changes. Manu suggested a change today and I've agreed with his change as well. I didn't realize there was one from Ted outstanding. What was it specifically?

Brent Zundel: Here is a link directly to his comment ^
… I assume it had been inadvertency overlooked

David Chadwick: [reads the comment/changes]
… Ah, okay. Right, I don't agree with that change, because I don't think holders do assemble credentials. I thought holders only assemble Verifiable Credentials into a Presentation, and then they made that into a Verifiable Presentation.

Brent Zundel: Excellent, we can move on. The notes from this meeting will automatically populate as a comment into that PR.

David Chadwick: Yes, I disagree with that change, but would like others to comment as well.

Brent Zundel: Folks would like to contribute: we encourage comments there.

@brentzundel
Copy link
Member

@TallTed were your concerns with this PR addressed to your satisfaction?

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

@brentzundel @David-Chadwick -- There are two changes that have not yet been applied, one from me, and one from @dlongley (and @msporny), as I've just flagged above.

brentzundel and others added 4 commits September 14, 2021 15:55
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
commit suggestion from @dlongley per @David-Chadwick's approval

Co-authored-by: Dave Longley <dlongley@digitalbazaar.com>
Signed-off-by: Brent Zundel <brent.zundel@gmail.com>
@brentzundel
Copy link
Member

@David-Chadwick I have committed the suggestions you were having trouble with from @TallTed and @dlongley in cc7befb and 4b50b3b, respectively.
Please let me know if this doesn't meet with your approval.

@msporny
Copy link
Member

msporny commented Sep 15, 2021

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

@msporny msporny merged commit 1dc83a0 into main Sep 15, 2021
@msporny msporny deleted the dwc-credentials branch September 29, 2021 23:41
@kdenhartog
Copy link
Member

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

@kdenhartog kdenhartog removed editorial Purely editorial changes to the specification. errata Erratum for a W3C Recommendation labels Nov 18, 2021
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.

None yet

7 participants