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

Define subschemas -> child instances in core #163

Closed
wants to merge 1 commit into from

Conversation

handrews
Copy link
Contributor

@handrews handrews commented Nov 23, 2016

This is a trial balloon for #161. While it seems like a radical change,
aside from using the "$" prefix for the keywords in question, there is no change
to any behavior.

This moves several keywords that control which subschemas apply
to which child instances from validation to core. The only
actual validation that these keywords defined was that all child
instances need to validate against all applicable subschemas.

This simple child validation principle has been moved up to
the general considerations section of the validation spec.

The moved keywords are now prefixed with "$", which is what we've
discussed doing for all core keywords (Another potential core
keyword would be "$data", for instance, as it would be used in
multiple vocabularies depending on which proposals we accept,
if any.)

As with "id" vs "$id" and the boolean forms of
exclusiveMinimum/Maximum, the old forms are still allowed but
expected to be removed before RFC. Migrating an old schema
to the new form is trivial, and was discussed before we decided
to rename "id". It is equally applicable here.

This is a trial balloon for fixing json-schema-org#161.

This moves several keywords that control which subschemas apply
to which child instances from validation to core.  The only
actual validation that these keywords defined was that all child
instances need to validate against all applicable subschemas.

This simple child validation principle has been moved up to
the general considerations section of the validation spec.

The moved keywords are now prefixed with "$", which is what we've
discussed doing for all core keywords (Another potential core
keyword would be "$data", for instance, as it would be used in
multiple vocabularies depending on which proposals we accept,
if any.)

As with "id" vs "$id" and the boolean forms of
exclusiveMinimum/Maximum, the old forms are still allowed but
expected to be removed before RFC.  Migrating an old schema
to the new form is trivial, and was discussed before we decided
to rename "id".  It is equally applicable here.
@awwright
Copy link
Member

I'm very skittish about expanding the meaning of core any beyond what's absolutely necessary for an application/schema+json parser to understand - namely, a way to name schemas, a way to reference other schemas, and a way to select a meta-schema.

One of the side effects this has is "allOf" is left behind and becomes harder to use.

I fully expect most meta-schemas to import JSON Schema Validation. Unless we can identify a problem with the approach that's been in use, how about we just formalize it and say "Metadata is applied in the course of validation to any instance successfully validated by the schema."

@handrews
Copy link
Contributor Author

@awwright I noticed the "allOf" problem as well and it bothered me but I couldn't easily figure out what to do with it. Decided to see if anyone else said anything :-)

how about we just formalize it and say "Metadata is applied in the course of validation to any instance successfully validated by the schema."

What's the exact definition of "metadata" here? Does that just mean anything other than validation?

BTW I'm not 100% on board with my own change here, I just felt that I needed to write it up as a diff in order to really wrap my head around it.

I feel like there's something to the subschema application thing that would benefit from being called out more clearly. The more I've separated (in my head) how these keywords work from how all of the other validation keywords work the more sense they (and associated issues around additionalProperties, etc.) make to me.

I don't have a real conclusion- I'm probably agreeing with you, actually, I just want to poke at the concept a bit more, if not at this approach.

@epoberezkin
Copy link
Member

I am strongly against this change. The difference between validation and controlling validation is perceptional, this is a change for the sake of change. It does not add any value

@handrews
Copy link
Contributor Author

@epoberezkin

  1. I seriously doubt this is going in. Maybe some ideas from it somewhere, but there's no need to get worked up about it.

  2. You keep talking about things as "perceptional". I don't think you are using the same definition of that word that I am, because in no case has your usage made any sense to me.

@epoberezkin
Copy link
Member

Just to make the objection clearer. I don't mind changing the language about how items etc work and clarifying that they essentially apply subschemas - it's kind of obvious but improving the language may make it clearer for some users. I am only against changing keywords.

@handrews
Copy link
Contributor Author

Just to make the objection clearer. I don't mind changing the language about how items etc work and clarifying that they essentially apply subschemas - it's kind of obvious but improving the language may make it clearer for some users. I am only against changing keywords.

Awesome. If there's anything that I would want to take from this experiment, that would be it. I'll probably play around with it tomorrow and see if there's any tweak that I think is a real improvement. Not a requirement for v6, but if I come up with something everyone likes it may go in :-)

What I wanted to do with this was take Austin's comment in #157 to its logical extreme and see where that put us. I've started labeling pull requests like this "trial balloons" ( #129 is another such trial balloon although I didn't call it that at the time). I generally don't expect trial balloon PRs to get merged, at least not without substantial work and quite likely not ever. I just find that making a concrete change to the spec can clarify things that are murky in issue discussions.

Sometimes these are even kind of "devil's advocate" proposals. Not unlike $combine which I mostly wrote up to show how difficult it would be.

@handrews
Copy link
Contributor Author

handrews commented Dec 5, 2016

I think this has gotten as much feedback as it needs. Language improvements that were discussed are better submitted as a new PR which I will look at eventually.

@handrews handrews closed this Dec 5, 2016
pdl pushed a commit to pdl/json-schema-spec that referenced this pull request Apr 11, 2017
Fixes old repo issues:
json-schema/json-schema#84
json-schema/json-schema#198

Previously submitted to the old repository by IvanGoncharov as commit 472c32b
and pull request json-schema-org#163 with original comment:

Added validation according to
http://json-schema.org/latest/json-schema-validation.html#anchor105

Merged into the old repo by geraintluff on May 8, 2015.
@handrews handrews deleted the recurse branch September 16, 2017 09:43
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