Skip to content

Conversation

@Sakurann
Copy link
Collaborator

More detailed and thorough Privacy Considerations section for ID-1 inspired by implementation experience and a lot of related specifications. Not perfect, but should be good enough to convey how serious we are about privacy.
intended to replace #187.

Copy link
Member

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

Approved, pending my suggestions being accepted.

Co-authored-by: Michael B. Jones <michael_b_jones@hotmail.com>
Co-authored-by: Joseph Heenan <joseph@heenan.me.uk>
Co-authored-by: Joseph Heenan <joseph@heenan.me.uk>
@Sakurann Sakurann requested a review from jogu February 1, 2024 17:45
Copy link
Member

@bc-pi bc-pi left a comment

Choose a reason for hiding this comment

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

Two minor nits on the latest text that I assume will be incorporated so approving.

Copy link
Contributor

@jogu jogu left a comment

Choose a reason for hiding this comment

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

Looks good to me! I agree with Brian's suggestions, and also suggested a few trivial grammar improvements.

@c2bo
Copy link
Member

c2bo commented Feb 2, 2024

The line breaks are somehow weird for this PR? Shouldn't for example

For example, when a military organization or a drug rehabilitation center issues a vaccine
credential, verifiers can deduce that the owner of the Wallet storing such Credential is a
military member or may have a substance use disorder.

be in one line like this?

For example, when a military organization or a drug rehabilitation center issues a vaccine credential, verifiers can deduce that the owner of the Wallet storing such Credential is a military member or may have a substance use disorder.

@jogu
Copy link
Contributor

jogu commented Feb 3, 2024

The line breaks are somehow weird for this PR?

The markdown has been wrapped, which is inconsistent with the rest of the spec - albeit that doesn't make any difference to the generated HTML.

@selfissued
Copy link
Member

The markdown has been wrapped, which is inconsistent with the rest of the spec - albeit that doesn't make any difference to the generated HTML.

The practice of putting an entire paragraph into a single source line results in MANY unnecessary merge conflicts. I support putting each sentence or long phrase into its own line - at least as we add new text.

That said, line breaks should be at the end of sentences and/or complete clauses so that odd line breaks don't make it hard to search for multiple-word terms such as "authorization server".

Copy link
Member

@peppelinux peppelinux left a comment

Choose a reason for hiding this comment

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

Approved considering Brian's suggestions taken into consideration

Copy link
Member

@c2bo c2bo left a comment

Choose a reason for hiding this comment

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

I agree with the suggestions of joseph & brian. Looks good to me apart from those minor changes

Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>
Co-authored-by: Joseph Heenan <joseph@heenan.me.uk>
Co-authored-by: Christian Bormann <8774236+c2bo@users.noreply.github.com>
Comment on lines 1516 to 1518
The Credential Issuer should obtain the End-User's consent before issuing Credential(s)
to the Wallet. It should be made clear to the End-User what information is being included in the
Credential(s) and for what purpose.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why lower-case should here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It used to be a MUST, we had a passionate discussion during the WG call based on a comment here and there were concerns with having a normative statements regarding consent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should all shoulds in this text be upper case SHOULD?

Copy link
Contributor

@tplooker tplooker left a comment

Choose a reason for hiding this comment

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

Minor nits but in general big supporter of this PR, thanks!

Co-authored-by: Joseph Heenan <joseph@heenan.me.uk>
@Sakurann Sakurann merged commit 67ad51c into main Feb 8, 2024
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.