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 @ from keywords #332

Merged
merged 6 commits into from
Sep 13, 2018
Merged

Remove @ from keywords #332

merged 6 commits into from
Sep 13, 2018

Conversation

iherman
Copy link
Member

@iherman iherman commented Sep 11, 2018

Removed the @ sign from the JSON-LD keywords @id, @type, @language, and @value, as agreed on a call for #312. (Not closing that issue, because there is still editorial work do to related to that issue. This meant

  • updated the JSON LD context file
  • updated the main document
  • updated all the manifest examples
  • updated all the diagrams
  • updated the wp.js script
  • updated the JSON schema files (@HadrienGardeur you should check that; e.g., the result is that the type term is now both a json schema keyword and a term in the data. I hope json schema can cope with that)

I have also

  • made some minor editorial cleanups
  • per request of @TzviyaSiegman I have removed the note on relating to the rel='canonical'.

Fix #324


Preview | Diff

@iherman
Copy link
Member Author

iherman commented Sep 11, 2018

Based on the discussion on #312 this PR does what we have decided. However, we may want to wait a bit to see if the changes in the JSON schema are fine or not (@HadrienGardeur). Then again, if there is an issue, that can be taken care of later...

@@ -223,4 +223,4 @@
}
},
"required": ["@context", "url", "@type"]

Choose a reason for hiding this comment

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

This should be changed from @type to type as well.

@iherman
Copy link
Member Author

iherman commented Sep 13, 2018

@HadrienGardeur thanks, I missed this one. I have changed that, too.

Merging and closing now.

@iherman iherman merged commit 4fce630 into master Sep 13, 2018
@iherman iherman deleted the remove-@ branch September 13, 2018 22:39
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.

3 participants