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

Add issue markers #1127

Closed
wants to merge 2 commits into from
Closed

Add issue markers #1127

wants to merge 2 commits into from

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented May 9, 2023

@@ -1336,7 +1336,7 @@ <h3>Types</h3>
<td>
<code>VerifiableCredential</code> and, optionally, a more specific
<a>verifiable credential</a> <a>type</a>. For example,<br>
<code>"type": ["VerifiableCredential", "UniversityDegreeCredential"]</code>
<code>"type": ["VerifiableCredential", "IdentityCredential"]</code>
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 made this more neutral, since our spec does not define UniversityDegreeCredential, the example here is misleading.

Obviously since the context is omitted, this term is expected to be defined as:

https://www.w3.org/ns/credentials/issuer-dependent#IdentityCredential

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 can revert this change if we get stuck on this attempt to improve this currently invalid example.

Copy link
Member

@msporny msporny May 13, 2023

Choose a reason for hiding this comment

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

IdentityCredential is going to confuse people given the charter in progress for the "Identity Credentials" work by Google, Apple, etc. We should avoid that terminology for two reasons:

  1. It will get confused by the upcoming W3C WG that will be similarly named.
  2. "Identity" is a lightning rod word that we should avoid in the specification, especially in examples.
  3. Using an issuer-dependent term as the core example is not something that will achieve consensus. Continuing to raise PRs that use issuer-dependent terms in core examples will continue to be objected to.

Alternate suggestion: ExampleDegreeCredential with a more comprehensive set of fixes in PR #1129.

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 think we are not a working group dedicated to the education vertical... and I find t he consistent bias towards academic use cases harmful to the standard... I hope we can find more neutral examples.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps ExampleAchievementCredential which could include merit badges from scouting-like organizations; trophies/medals from athletic competitions; "top salesperson" from employer, professional org, etc.; and so on?

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.

This PR isn't just adding issue markers, it's attempting to also change the core example in the specification to language that is going to be problematic given the new W3C WG being formed around identity credentials. The naming is going to create unnecessary conflict.

@@ -1336,7 +1336,7 @@ <h3>Types</h3>
<td>
<code>VerifiableCredential</code> and, optionally, a more specific
<a>verifiable credential</a> <a>type</a>. For example,<br>
<code>"type": ["VerifiableCredential", "UniversityDegreeCredential"]</code>
<code>"type": ["VerifiableCredential", "IdentityCredential"]</code>
Copy link
Member

@msporny msporny May 13, 2023

Choose a reason for hiding this comment

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

IdentityCredential is going to confuse people given the charter in progress for the "Identity Credentials" work by Google, Apple, etc. We should avoid that terminology for two reasons:

  1. It will get confused by the upcoming W3C WG that will be similarly named.
  2. "Identity" is a lightning rod word that we should avoid in the specification, especially in examples.
  3. Using an issuer-dependent term as the core example is not something that will achieve consensus. Continuing to raise PRs that use issuer-dependent terms in core examples will continue to be objected to.

Alternate suggestion: ExampleDegreeCredential with a more comprehensive set of fixes in PR #1129.

<code>"type": "StatusList2021Entry"</code>
<code>"type": "StatusList2021Entry"</code>.

<p>See <a href="https://www.w3.org/TR/vc-status-list/">vc-status-list</a>.</p>
Copy link
Member

Choose a reason for hiding this comment

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

This should be an informative ref to the status list spec.

@@ -1401,6 +1406,8 @@ <h3>Types</h3>
<td>
A valid terms of use <a>type</a>. For example,<br>
<code>"type": "OdrlPolicy2017"</code>)

<p class="issue" data-number="1123"></p>
Copy link
Member

@msporny msporny May 13, 2023

Choose a reason for hiding this comment

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

Not sure that I'm a fan of adding issue markers throughout the spec for every issue. We have 65+ issues where we'd have to do that, only to go back through and remove them. It's unnecessary churn for the Editors (without good reason for the churn), and this isn't being done consistently, AFAICT, from the latest issue marker additions.

Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that issue markers are appropriate for significant publication milestones, like CRs. Less so, though arguable, for FPWDs. Far less so for ED-per-commit and/or daily-ED, though editor's prerogative probably holds.

Given that, I'd suggest that there be some rule of thumb that issue markers be added based on exceeding some number of comments and/or duration of discussion (I don't have a suggestion for either trigger), on the presumption that the issue is less likely to be closed quickly, and so more likely to persist beyond the next milestone publication and/or transition, which is likely to bring more unfamiliar readers for whom the issue markers are intended.

@OR13 OR13 closed this May 22, 2023
@msporny msporny deleted the feat/add-issuer-markers branch July 27, 2023 21:07
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.

None yet

5 participants