Skip to content

fix name id #1073

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

Closed
wants to merge 2 commits into from
Closed

fix name id #1073

wants to merge 2 commits into from

Conversation

suutaku
Copy link

@suutaku suutaku commented Mar 31, 2023

typo


Preview | Diff

@w3cbot
Copy link

w3cbot commented Mar 31, 2023

iherman marked as non substantive for IPR from ash-nazg.

Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

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

The hash at the top of the section should be fixed along with this. Notably, the use of type with xsd:string is also unnecessary and could be simplified (same applies to description).

index.html Outdated
Comment on lines 4924 to 4927
"name": {
"@id": "https://schema.org/description",
"@id": "https://schema.org/name",
"@type": "http://www.w3.org/2001/XMLSchema#string"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems this should just be:

"name": "https://schema.org/name",

Copy link
Member

Choose a reason for hiding this comment

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

Yes, agree (and change applied).

@OR13
Copy link
Contributor

OR13 commented Apr 3, 2023

hmm #1053

Feels like maybe a data-include would have been a better approach to this... Why is the whole context embedded in html?

@msporny
Copy link
Member

msporny commented Apr 7, 2023

@OR13 wrote:

Feels like maybe a data-include would have been a better approach to this...

Good point, I had missed that this PR was updating the index.html (and had presumed it was updating the context).

Commit 52030d3 uses data-include now.

Commit 14beefb applies the parts of this PR that didn't make it into the base v2 JSON-LD context.

That effectively processes this PR, so even though I'm closing this PR, the updates have been made to the spec and base JSON-LD Context. Thank you @suutaku for finding the issue.

Why is the whole context embedded in html?

During the v1.0 work, there were a number of people in the group that felt that directly embedding the context in the spec would give it more weight than referring to a file outside the spec (that might change). It also gave implementers a value that they could directly copy/paste, verify the hash of, and then embed in their software. Some were concerned that if the v1 context were modified/changed, that we wouldn't have a spec to refer back to with what the VCWG intended at the time (this viewpoint ignores the fact that we have source control). In practice, that file outside of the core spec didn't change, and perhaps there is no longer a need to have the context in the spec given that we have a hash for the URL at which it's published. I'd be supportive of removing it from the spec and just referring to it by hash (noting that the hash will need to be updated as we go into PR to ensure we lock the correct version in).

@msporny msporny closed this Apr 7, 2023
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.

6 participants