Various language improvements #236

awwright · 2017-01-27T22:19:00Z

No description provided.

handrews

Nice work!

While I still think we need to sort out a lot of things about what "default" really means, I agree that "has the same behavior" is an improvement. And I don't want to delay Draft 06 for a more thorough change, so I'm in favor of this.

I have a couple of other minor comments, mostly around how schemas can now be booleans and not just objects.

handrews · 2017-01-27T22:32:18Z

jsonschema-validation.xml

@@ -167,8 +162,8 @@

            <section title="Keywords and instance primitive types">
                <t>
-                    Most validation keywords only limit the range of values within a certain primitive type.
-                    When the primitive type of the instance is not of the type targeted by the keyword, the
+                    Most validation keywords only exclude a range of values within a certain primitive type.


What about

Most validation keywords only constrain values within a certain primitive type.

We've often talked about validation as a constraint system, and I think it feels more intuitive than talking about exclusion.

The language is sort of tricky overall... revised

handrews · 2017-01-27T22:36:01Z

jsonschema-validation.xml

@@ -575,21 +559,23 @@
                    validates against the provided schema.
                    Note the property name that the schema is testing will always be a string.
                </t>
+                <t>
+                    A missing keyword has the same behavior as an empty schema.


Should we specify this as a true schema instead of an empty schema? Admittedly, the behavior is the same but true conveys the intent of permitting everything more clearly.

I think so too

"empty schema" is language that was used before (that this PR doesn't introduce), can we work on this in a new PR and call this one good?

If empty schema vs true is the only concern, I endorse doing that in a follow-up PR. We should check for it in more places than just these lines, and that will be trivially easy to check up on on its own.

Yeah, that's the thing, it's used quite a few times

handrews · 2017-01-27T22:37:24Z

jsonschema-validation.xml

                </t>
                <t>
-                    Elements of the array MUST be objects. Each object MUST be a valid JSON
-                    Schema.
+                    Elements of the array MUST be objects.


I think we just want to say

Each element of the array MUST be a valid JSON Schema.

in order to allow boolean schemas as well.

handrews · 2017-01-27T22:38:05Z

jsonschema-validation.xml

                </t>
                <t>
-                    Elements of the array MUST be objects. Each object MUST be a valid JSON
-                    Schema.
+                    Elements of the array MUST be objects.


Same thing:

Each element of the array MUST be a valid JSON Schema.

I thought that's what we did at first and then it was changed back for some reason... idk. I'll do this though.

I think maybe I changed it like that in a PR that I ended up dropping? The boolean schema thing went through a few variations before we settled on it. Whatever happened, I think just saying "valid JSON Schema" is the best option- we already define what that means in an obvious place (the core spec).

handrews · 2017-01-27T22:38:40Z

jsonschema-validation.xml

@@ -664,19 +650,21 @@

            <section title="not">
                <t>
-                    This keyword's value MUST be an object. This object MUST be a valid JSON
-                    Schema.
+                    This keyword's value MUST be an object.


Same thing:

This keyword's value MUST be a valid JSON Schema.

Of course, {"not": true} doesn't make much sense, I think it should be syntactically valid.

handrews

Looks great! Let's let @Relequestual and @epoberezkin weigh in since they had somewhat different opinions of the "same behavior" part, but this looks good to me.

handrews · 2017-01-31T06:42:24Z

@epoberezkin , @Relequestual are y'all OK with this? I'd like to get these fixes merged if so. There is no behavior change so it doesn't need to sit open for weeks.

Relequestual · 2017-01-31T16:50:38Z

I need more time to review this PR. I feel in some places it's actually more confusing, and I have ideas how to fix that, but I can't do it right now.

Relequestual · 2017-02-01T13:31:35Z

jsonschema-validation.xml

@@ -575,21 +559,23 @@
                    validates against the provided schema.
                    Note the property name that the schema is testing will always be a string.
                </t>
+                <t>
+                    A missing keyword has the same behavior as an empty schema.


I find the phrasing "A missing key word...", does not make me think that the missing key word in question is THIS key word (as in, the key word in where the phrase is being used).
I would much rather prefer something like "Omitting this key word has the same effect as...". I feel the intent is far clearer.

If it's just me that feels this way, please say.

That's exactly the word I was looking for, actually

awwright · 2017-02-01T13:44:49Z

@Relequestual How's it look now

Relequestual

All good in my books! =D

Relequestual · 2017-02-01T15:09:58Z

As such... I'm merging!

Various language improvements

0667b89

handrews suggested changes Jan 27, 2017

View reviewed changes

awwright added 2 commits January 27, 2017 15:52

Validation: Allow allOf/anyOf/oneOf/not to specify non-object schemas

a961689

Validation: Reword that sentence again

8947e93

handrews approved these changes Jan 27, 2017

View reviewed changes

Validation: Fix "same as" sentence for "items" too

5246792

Relequestual reviewed Feb 1, 2017

View reviewed changes

Validation: s/A missing/Omitting this/

d4122bf

Relequestual approved these changes Feb 1, 2017

View reviewed changes

Relequestual merged commit 703c107 into json-schema-org:master Feb 1, 2017

awwright deleted the aaa-a branch February 1, 2017 15:56

Uh oh!

Various language improvements #236

Various language improvements #236

Uh oh!

Conversation

awwright commented Jan 27, 2017

Uh oh!

handrews left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awwright Jan 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

handrews left a comment

Choose a reason for hiding this comment

Uh oh!

handrews commented Jan 31, 2017

Uh oh!

Relequestual commented Jan 31, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awwright commented Feb 1, 2017

Uh oh!

Relequestual left a comment

Choose a reason for hiding this comment

Uh oh!

Relequestual commented Feb 1, 2017

Uh oh!

Uh oh!

awwright Jan 31, 2017 •

edited

Loading