Skip to content

Added new "requiredProperties" keyword to supersede the "required" keyword #681

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

Closed
wants to merge 2 commits into from

Conversation

jgonzalezdr
Copy link
Contributor

Added boolean values to "required" such that when set to true all the properties present in "properties" are required (related to #659).

@awwright
Copy link
Member

Unfortunately this was previously defined as a boolean, but with different semantics, and this behavior is implemented in more than a few packages for reverse compatibility, e.g. tdegrunt/jsonschema.

I've once suggested having a "requiredProperties" keyword that functions the same as "properties" does, except that all the keywords there are also required. There was an argument against this too and I forget what it was.

@jgonzalezdr
Copy link
Contributor Author

Maybe the solution could be creating a new "requiredProperties" keyword with the semantic that I just proposed above, which by the way would have a "naming convention" homogeneous with the rest of validation keywords for properties. Then the "required" keyword could be deprecated at some point. Anyway both could coexist during some time if needed.

@jgonzalezdr jgonzalezdr changed the title Added boolean values to "required". Added new "requiredProperties" keyword to supersede the "required" keyword Nov 23, 2018
@handrews
Copy link
Contributor

@jgonzalezdr we have a good discussion going on in issue #659, so I'd like to close this until that discussion resolves. Otherwise, people will think this is the approved solution and review it as such.

If we decide on this direction, we will re-open this PR. But the requiredProperties solution also seems worth discussing. Let's resolve that before having an active PR attracting attention.

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