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

Issue 446 version announcement #473

Merged
merged 3 commits into from
Apr 17, 2017
Merged

Conversation

gkellogg
Copy link
Member

@gkellogg gkellogg commented Apr 8, 2017

Adds announcement requirement and tests as required in #446.

…ich was not the point of the test and causes errors in JSON parsers that are running in a strict mode.
…either using `processingMode` API option set to "json-ld-1.1", or `"@Version": 1.1` in the first encountered JSON Object context, otherwise processing mode set implicitly to "json-ld-1.0". Added checks with expected errors in context processing if new features used in context with the wrong processing mode. Also adds compact/expand tests to check for proper error reporting.

Fixes #446.
@gkellogg gkellogg added this to the JSON-LD 1.1 milestone Apr 8, 2017
@gkellogg gkellogg self-assigned this Apr 8, 2017
@@ -0,0 +1,5 @@
{
"@context": {
"@version": 1.1
Copy link
Member

Choose a reason for hiding this comment

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

Do we want this to be a number (float/double) or a string? Expressing version attributes as strings seems to be fairly common practice.

Copy link
Member Author

Choose a reason for hiding this comment

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

@lanthaler suggested a number, as using a string would be silently ignored by most 1.0 processors; the point is to use something that would cause a 1.0 processor to halt, thus the number.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right, I forgot about that. Bummer.

The processing mode defines how a JSON-LD document is processed.
By default, all documents are assumed to be conformat with
<a data-cite="JSON-LD-20140116">JSON-LD 1.0</a> [[!JSON-LD-20140116]]. By defining
a different version using the <code>@version</code> member in a
Copy link
Member

Choose a reason for hiding this comment

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

I suppose there may be good reasons for decoupling the version from the processing mode, but figured I'd throw out the idea that we could do @processingMode instead of @version as a means by which to set the preferred processing mode via the @context -- rather than inferring it from @version.

Copy link
Member Author

Choose a reason for hiding this comment

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

That thought occurred to me, but it would be more natural, then, to say "@processingMode": "json-ld-1.1", which would pass a 1.0 processor silently.

An alternative suggested by @lanthaler was to use something like "@processingMode": ["json-ld-1.1"], which would error by a 1.0 processor, would would allow the symmetry with the API mode. Personally, I preferred using the simpler numeric value, but I could be convinced to go the other way.

Copy link
Member

Choose a reason for hiding this comment

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

I don't feel that strongly, so I'm ok with going forward with the numeric proposal for now given its necessity for causing a 1.0 processor to halt.

Copy link
Member

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

LGTM -- I only did a quick pass over as I'm time limited. We may want to be a little more clear that the version value is a JSON Number. I think this is the best path forward we've come up with so far so I'm +1 to merge. We can make further changes as needed after more feedback comes in.

@gkellogg
Copy link
Member Author

gkellogg commented Apr 8, 2017

Thanks @dlongley: I'll add some additional text to clarify that the value is the JSON number 1.1.

@gkellogg gkellogg merged commit 6d014ea into master Apr 17, 2017
@gkellogg gkellogg deleted the issue-446-version-announcement branch April 17, 2017 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants