Add the "contains" keyword for array instances. #113
Conversation
This addresses the enhancement requested in issues json-schema-org#32 and json-schema-org#63. Only the single-schema form from json-schema-org#63 is added here as the multi-schema form did not gather significant support in the absence of a clear use case.
| <t> | ||
| An array instance is valid against "contains" if at least one of | ||
| its elements is valid against the given schema. | ||
| </t> |
There was a problem hiding this comment.
If I want an array to contain an item, I would think that means empty array should always be invalid.
I don't see any reason to have a special behavior for an edge case.
Is there a case I would want to treat an empty array instance as valid?
There was a problem hiding this comment.
The "or array is empty" is easily handled by:
{
"type": "array",
"items": {...},
"oneOf": [
{"contains": {...}},
{"maxItems": 0}
]
}There was a problem hiding this comment.
It follows from contains being equivalent to:
{
"not": {
"items": {
"not": {... whatever ...}
}
}
}There was a problem hiding this comment.
yes, ok, should have at least one item :)
There was a problem hiding this comment.
No, an empty array fails that schema, because an empty array passes the schema "items": {"not": {...}}, so the outer not causes it to fail overall.
|
@awwright @Relequestual I think all outstanding concerns here are resolved, any chance of getting this merged? |
|
I'm in favour of saying, with 3 change approvals, merging is OK to go. |
Do you mean I need to get two more formal approvals? @awwright , @epoberezkin ? |
|
Actually, given the "official" people involved, that could be a bit tricky. Can non members submit reviews? |
|
@Relequestual : Yup, as you probably noticed because I just tried it out by commenting on your PR :-) |
|
Let's do this. JSON Schema can do this functionality without |
|
OK. if a contributor reviews and agrees then I'll merge! |
|
@epoberezkin , @awwright any chance I can convince either of you to be approver #3? |
|
I'd normally wait 14 days but this seems relatively uncontroversial |
|
Thanks @awwright . I had figured since this was was a pretty direct implementation of something that had gotten approval on the issue, that the 14 day counter included the period of agreement on the issue. If you want pull requests to always stay open for 14 days that's good to know. I'd like to try to get the requisite approval comments as soon as possible but I'm fine with leaving things open for a minimum period. We should document that somewhere, perhaps a "contributors" section of the site? Or a wiki page in this repo? |
|
I would personally like to see this info (on what will happen and how) in the repo readme. Even if JUST "we will wait 14 days before merging, if we like it" |
|
@Relequestual I filed issue #137 to collect stuff like "we will wait 14 days" before merging. I'm tired of getting criticized for not following rules that aren't written down :-( |
|
I was explaining what I do to help manage expectations, it's not really a On Nov 8, 2016 11:42 AM, "Henry Andrews" notifications@github.com wrote:
|
Whether it's a "rule" or some other word is not the point. The point is that it's not documented, and there are a lot of these things that are not documented. Tripping over them when you suddenly say that something I've been doing is counter-productive or pointless or trying to move too fast (and really I'm not trying to rush anything) is very frustrating. I could have just not spent time on the thing you don't want people to work on or aren't going to look at anytime remotely soon. And I don't want to be pestering people when there's a minimum deadline that hasn't been met. Etc. etc. etc. |
This addresses the enhancement requested in issues #32 and #63.
Only the single-schema form from #63 is added here as the multi-schema form
did not gather significant support in the absence of a clear use case.