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

Remove broken links #1066

Closed
wants to merge 1 commit into from
Closed

Remove broken links #1066

wants to merge 1 commit into from

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Mar 15, 2023

Relies on issuer defined terms, which is the simplest way to define terms, and does not require the implementer to download several separate JSON-LD context files to make use of the examples in the spec.

^ this link is 404, and causes the examples in the spec to be invalid.


Preview | Diff

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.

Objection to the PR, it would make just about every example in the spec use the "undefined terms" property, which would set a bad precedent.

Alternatives that could achieve consensus include:

  1. Create a "Getting Started" section that outlines how to get started by using the base context, and then layer semantic meaning after the developer has stabilized the terms they want to use.
  2. Use JSON-LD Contexts that are being used in pilot and production settings.
  3. Upgrade the base context to include terms like "name" and "website", or other generally useful properties for a credential subject, which could then be used in examples.
  4. Use JSON-LD Contexts from the VC Specs Directory.

The options above are being discussed in issue #1071 and a PR here #1064 (comment).

It is highly unlikely that there will be consensus to downgrade almost every example in the specification to, what is effectively, a bad practice in general.

@OR13
Copy link
Contributor Author

OR13 commented Mar 16, 2023

Objection to the PR, it would make just about every example in the spec use the "undefined terms" property, which would set a bad precedent.

Actually this fixes that problem (which exists in the spec today), it defines the terms using the default vocab.

The current examples are malformed, and this PR is the simplest possible fix to them (since if defines their terms using the default context).

Alternatives that could achieve consensus include:

  1. Create a "Getting Started" section that outlines how to get started by using the base context, and then layer semantic meaning after the developer has stabilized the terms they want to use.

This does not fix the problem that the examples in the spec are malformed currently.

Happy to address this in a separate PR.

  1. Use JSON-LD Contexts that are being used in pilot and production settings.

Introduces further bias, and makes the examples in the spec more complicated.

  1. Upgrade the base context to include terms like "name" and "website", or other generally useful properties for a credential subject, which could then be used in examples.

This seems like maybe a path forward, make the examples simpler, so you don't need context complexity in them.

You can also use the issuer defined terms to achieve this, that is exactly what the current PR does.

  1. Use JSON-LD Contexts from the VC Specs Directory.

Introduces further bias, and makes the examples in the spec more complicated.

Copy link
Contributor

@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.

I agree that all the hyperlinks in our specification should be valid.

@OR13
Copy link
Contributor Author

OR13 commented Apr 11, 2023

@Sakurann @brentzundel I would like some call time to discuss this and the JSON Schema PR:

@msporny msporny added DO NOT MERGE PR contains something that should not be merged. discuss labels Apr 16, 2023
@OR13
Copy link
Contributor Author

OR13 commented Apr 17, 2023

I am fine with the working group adding links that have consensus back to the examples, we can keep having that conversation here:

#1071

It seems better to remove broken links first, and then add non broken links back.

The links that are removed are not required and the examples are non normative.

@TallTed
Copy link
Member

TallTed commented Apr 18, 2023

It seems better to remove broken links first, and then add non broken links back.

It seems to me harder to discover where those broken links were once they're removed, so I'd prefer to keep each (easy to locate) broken link until it is corrected, unless/until there's a determination that that link is not going to be corrected, for whatever reason.

@Sakurann
Copy link
Contributor

Sakurann commented May 3, 2023

It seems to me harder to discover where those broken links were once they're removed,

why does the WG need to discover old broken links? the one who is interested in fixing it will know where they are

@OR13
Copy link
Contributor Author

OR13 commented May 3, 2023

I'm closing this PR in favor of https://github.com/w3c/vc-data-model/pull/1111/files

@OR13 OR13 closed this May 3, 2023
@iherman
Copy link
Member

iherman commented May 3, 2023

The issue was discussed in a meeting on 2023-05-03

  • no resolutions were taken
View the transcript

3.4. (pr vc-data-model#1066)

See github pull request vc-data-model#1066.

Brent Zundel: next up is PR 1066.
… request for changes from manu but not here.

Orie Steele: goals.
… technical recommendation contains malformed JSON-LD.
… PR intends to remove 404.
… manu and dave and potentially others have a model of what example they'd like to see examples in the core spec.
… examples that contain multiple contexts.
… but i don't like examples with multiple contexts.
… just having some conversation on different opinions on examples.

Dave Longley: just quick clarification.
… we should have examples of both.
… first examples should be issuer independent vocabs.

Orie Steele: I am also in favor of examples of both, as I stated.

Dave Longley: and then issuer dependent vocabs but we should also say that this could be a risk.

Orie Steele: Not sure I agree regarding the "market failures" statement.

Orie Steele: but i look forward to reviewing text that elaborates on that.

Dave Longley: we should fix context instead of deleting them from examples.

Brent Zundel: two paths possible.
… one PR gets merged and then issue gets created.

Orie Steele: There are already open issues that track handling examples.

Brent Zundel: agree that the links in the examples should work.
… the other option is to create another PR that fixes the broken links.

Dave Longley: .
… another PR somewhere.
… to define an examples v2 context.
… that has examples vocab in it.
… not looked into it recently.
… but if gets traction.
… then we can merge that.
… because fixes broken links.
… and then can iterate on context.

See github pull request vc-data-model#1110.

Dave Longley: to improve examples.

Orie Steele: ^ that is the PR he is refering too.

Brent Zundel: orie?

Orie Steele: haven't had a chance to review it.

@brentzundel
Copy link
Member

I'm closing this PR in favor of https://github.com/w3c/vc-data-model/pull/1111/files

did you mean #1110 ?

@msporny msporny deleted the fix/broken-links branch July 27, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss DO NOT MERGE PR contains something that should not be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants