Skip to content

Clarify: all applicators collect annotations, including "contains" #768

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
handrews opened this issue Jul 25, 2019 · 4 comments · Fixed by #769
Closed

Clarify: all applicators collect annotations, including "contains" #768

handrews opened this issue Jul 25, 2019 · 4 comments · Fixed by #769
Labels
clarification Items that need to be clarified in the specification core

Comments

@handrews
Copy link
Contributor

As noted in #766, we need to emphasize how applicators and annotation collection works, in particular that an applicator like contains does collect annotations, which means that when annotation collection is enabled, you cannot short-circuit the evaluation. I think we note this with anyOf but not contains. Those are the only two applicators that combine validation results with OR and could therefore be short-circuited when not collecting annotations.

@gregsdennis
Copy link
Member

gregsdennis commented Jul 25, 2019

All annotations? What about not? It's been made clear that it will never return annotations due to its nature.

Otherwise I think this is a good idea.

@handrews
Copy link
Contributor Author

@gregsdennis annotations are still dropped whenever validation fails (and it's impossible to evaluate not without either failing the subschema, before not passes, or passing the subschema, in which case not inverts the validation result to fail)- this is a note to myself to fix a thing, don't treat it as spec. NOTHING IS CHANGING HERE.

@handrews
Copy link
Contributor Author

@gregsdennis ok apparently I'm a lying lier who lies because contains currently says it does not collect annotations 😮

I'm 99% sure that is because we didn't have any sort of output format when I first wrote the spec for that keyword, and it wasn't clear how to make use of them if they were collected. That is no longer the case, so I'm writing the PR to change it to collect annotations and be a "normal" applicator.

We can debate whether that is the right option on the PR 😄

@karenetheridge
Copy link
Member

clerical error? This issue is in milestone draft-09, but the PR that closed it is in milestone draft-08 (which maybe should be subtitled ".. (2019-09)").

@Relequestual Relequestual removed this from the draft-09 milestone Sep 18, 2020
@gregsdennis gregsdennis added clarification Items that need to be clarified in the specification and removed Type: Maintenance labels Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Items that need to be clarified in the specification core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants