Skip to content

Flip protocol-relative URI Reference (not a URI) #156

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

Merged
merged 1 commit into from
Feb 3, 2017

Conversation

awwright
Copy link
Member

Affected test was added in #87

@awwright awwright added the bug A test is wrong, or tooling is broken or buggy. label Jan 26, 2017
@awwright
Copy link
Member Author

@epoberezkin I can see how draft-03 makes things a little ambiguous, but draft-fge-json-schema-validation-00 specifically says

A string instance is valid against this attribute if it is a valid URI, according to [RFC3986].

I don't think we should be adding any new tests to the expired suites, but certainly we can fix broken ones.

In all three files, that schema-test tests two URI References, one is valid and one is invalid. Well it has to be one or the other...

@epoberezkin
Copy link
Member

@awwright got it, no problem, just wanted to clarify.

I don't think we should be adding any new tests to the expired suites, but certainly we can fix broken ones.

I actually though it could be good to add a couple of tests to draft 4 - there are plenty of badly supported cases... The motivation for it is that you can diff between 4 and 6 folders and see that it is not a change a spec but just an additional test to both the latest and previous spec version.

What do you think?

@awwright
Copy link
Member Author

@epoberezkin The I-D still isn't out yet, get it in quick? hehe

In any event, it's just my opinion.

@awwright
Copy link
Member Author

@Julian Anything left to merge this?

@Julian
Copy link
Member

Julian commented Jan 27, 2017

@awwright nope already reviewed :)

@Relequestual
Copy link
Member

I was about to merge, but can I request you change the commit message to reference the issue and this PR please? That way, if anyone sees the change, looks it up on github, and wants a place of reference to understand why the change happened, and that they can challenge it / raise an issue with it, then it would make doing so far easier. Make sense?

@awwright
Copy link
Member Author

@Relequestual @Julian can do that in the merge message. Actually, that should be the default behavior.

@Julian Julian merged commit a599e1e into json-schema-org:master Feb 3, 2017
Julian added a commit that referenced this pull request Feb 3, 2017
Plenty of confusion still about Draft 3/4's URI, but
that's already being addressed in Draft 6.

After this PR, the tests here are at least internally
consistent (at this point they all consider only URIs
not URI references).

Refs: #58, #77, #156

* awwright/master:
  Flip protocol-relative URI Reference (not a URI)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A test is wrong, or tooling is broken or buggy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants