Skip to content

GSD tweaks for 2020 12 patch #1238

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 29 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8c42999
Clarify vocab specs needn't be formal or published.
handrews May 11, 2021
6a4183e
Remove redundant meta-schema update process notes
handrews May 11, 2021
7f51e1c
Merge pull request #1101 from handrews/meta-schema-process
handrews Jun 1, 2021
f42ca9f
Merge pull request #1103 from handrews/vocab-formality
handrews Jun 1, 2021
770e7f5
Clarify the origin of contentEncoding (#1117)
handrews Nov 9, 2021
8eb04a8
Nitpick content-encoding. See #1100 (#1150)
ioggstream Nov 19, 2021
0f43deb
Add missing closing curly brace to D.2. example
jviotti Dec 30, 2021
e0c6563
mention that annotations from these keywords affect unevaluated*
karenetheridge May 31, 2021
cff5ac1
remove uses of "production" when discussing format rules
karenetheridge May 31, 2021
44f5b82
rewording of sections on "contains", "maxContains", "minContains"
karenetheridge May 31, 2021
53736b4
"contains" did not annotate before 2020-12
karenetheridge May 31, 2021
48736f7
$recursiveRef -> $dynamicRef was not just a simple keyword renaming
karenetheridge May 31, 2021
6853f30
improve wording
karenetheridge Feb 4, 2022
8d59764
no shouty
karenetheridge Feb 4, 2022
371dae1
Remove remaining media-type param references
jdesrosiers Feb 3, 2022
db65da8
Clarify that plain name fragments are neither canonical or non-canoni…
Relequestual Mar 14, 2022
4f9e8be
Clarify various things about canonical URIs
handrews May 14, 2021
afca53d
Update based on review feedback.
handrews Jul 13, 2021
c186b9a
Add CREF about ambiguous behaviour of additionalProperties
Relequestual Mar 30, 2022
f13350a
Remove detail about ambiguity and reasoning. Link out to pending ADR
Relequestual Apr 8, 2022
e8a6028
Add folder for architectural decision records (ADRs)
Relequestual Apr 8, 2022
26bfeb8
Tidy initial links in an ADR
Relequestual Apr 20, 2022
74a4e7e
Further tidy
Relequestual Apr 20, 2022
98e3ee7
Change date of validation spec to 2022. Year has to be current or xml…
Relequestual Apr 28, 2022
56ebcd0
Update draft identifiers for 2020-12 patch release
Relequestual Apr 28, 2022
9afa06b
Changes ADR phrasing to improve readability
Relequestual May 10, 2022
859ca97
Add changelog for 2020-12 patch 1 update
Relequestual May 19, 2022
438866e
Updated ADR status to accepted and updated date to current, reflectin…
Relequestual May 19, 2022
4c83377
add clarification that minContains:0 also causes contains to pass
gregsdennis May 26, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# Acknowledge ambiguity in additionalProperties behaviour and fix after patch release

* Status: accepted
* Deciders: @relequestual @gregsdennis, @jdesrosiers, @karenetheridge
* Date: 2022-05-19

Related...

Issue: https://github.com/json-schema-org/json-schema-spec/issues/1172

Discussion: https://github.com/json-schema-org/community/discussions/57

Pull Request: https://github.com/json-schema-org/json-schema-spec/pull/1203

## Context and Problem Statement

When we changed the specification to use annotations as the context in which some keywords behave, we included a clause that allowed implementations which didn't use annotations to optimize the processing of `additionalProperties` in another way which produces the same effect as the prior behaviour.
This section created an ambiguity in terms of the resulting output format, but not validation.

We needed to decide on how to proceed for the patch release of the 2020-12 version of the specification.

The two above links are to a GitHub Discussion and a GitHub Issue detailing the problems.
Details with an example of the problem can be seen in the Discussion's opening post specifically.

## Decision Drivers <!-- optional -->

* The "patch release" should not change anything functionally
* Annotations as they are, are confusing to users, implementers, and specification editors alike
* Patch release is behind schedule
* There are currently no tests for the output format
* It's hard to see any immediate consensus on changing the annotation based behaviour

## Considered Options

* [Leaving it "as is" and do nothing](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1413777)
* [Pick one](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1416683) of the behaviours
* [Revert back to draft-07 behaviour](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1453723)
* [Reinterpret how we understand annotation collection](https://github.com/json-schema-org/json-schema-spec/issues/1172#issuecomment-1049686478) to allow reading annotations within the same schema object regardless of assertion results
* [Acknowledge and accept that two approaches and results are allowable](https://github.com/json-schema-org/community/issues/161#issue-1173742930)
* Redefine annotation collection behaviour and/or how `additionalProperties` works

## Decision Outcome

Chosen option: "Acknowledge and accept that two approaches and results are allowable", because

* Leaving it "as is" will continue to cause confusion
* The change is non-functional which is required for the patch release
* The patch release is behind schedule
* Finding consensus of other solutions proved to be difficult
* There's no test suite for the output format, so it's not easy to see unintended consequences of a functional change
* We need to properly re-evaluate annotation collection and how annotations are used by other keywords

### Positive Consequences

* Patch release can move forward
* Validation result is not impacted
* Confusion is at least seen and acknowledged
* Implementations which pick either approach are seen to be compliant

### Negative Consequences

* May have an impact for downstream tools which process full output data
* A test suite (not yet developed) which covers this situation needs to allow for multiple valid answers

## Pros and Cons of the Options

### Leaving it "as is" and do nothing

Agree to do nothing and hope for the best. There isn't any downstream tooling yet anyway.

* Good, because no functional change
* Good, because no impact on downstream tooling
* Bad, because leaves a known ambiguity in the specification

### Pick one / Revert to draft-07 behaviour / Reinterpret annotation collection

* Good, because ambiguity is removed
* Good, because not many tools will be effected
* Bad, because it can be seen as a functional change (not really allowed for the patch release)
* Bad, because it can break existing implementations and downstream tools
* Bad, because without a test suite it's hard to see unexpected consequences

## Links

* Issue: [Ambiguous behaviour of additionalProperties when invalid](https://github.com/json-schema-org/json-schema-spec/issues/1172)
* Discussion: [The meaning of "additionalProperties" has changed](https://github.com/json-schema-org/community/discussions/57)
* Resolving Pull Request: [Add CREF about ambiguous behaviour of additionalProperties](https://github.com/json-schema-org/json-schema-spec/pull/1203)
* Alternative solution proposal: [Resolve contradictions in the described behaviour of "additionalProperties" and "items"](https://github.com/json-schema-org/json-schema-spec/pull/1154)

* [Result of discussing](https://github.com/json-schema-org/json-schema-spec/issues/1172#issuecomment-1063962900) on an Open Community Working Meeting call - @jdesrosiers proposed a less controversial and more agreeable solution to add a comment that both are allowable
* [Related GitHub Discussion](https://github.com/json-schema-org/community/discussions/67) on alternative behaviour for `unevaluated*` keywords
15 changes: 15 additions & 0 deletions adr/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Architectural Decision Log

This log lists the architectural decisions for the JSON Schema specification.

<!-- adrlog -- Regenerate the content by using "adr-log -i". You can install it via "npm install -g adr-log" -->

* [ADR-2022-04-08](2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md) - Acknowledge ambiguity in additionalProperties behaviour and fix after patch release

<!-- adrlogstop -->

You can find the ADR for using ADRs in our [community repo ADR log](https://github.com/json-schema-org/community/tree/HEAD/docs/adr).

For new ADRs, please use [template.md](template.md) as basis.
More information on MADR is available at <https://adr.github.io/madr/>.
General information about architectural decision records is available at <https://adr.github.io/>.
72 changes: 72 additions & 0 deletions adr/template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# [short title of solved problem and solution]

* Status: [proposed | rejected | accepted | deprecated | … | superseded by [ADR-0005](0005-example.md)] <!-- optional -->
* Deciders: [list everyone involved in the decision] <!-- optional -->
* Date: [YYYY-MM-DD when the decision was last updated] <!-- optional -->

Technical Story: [description | ticket/issue URL] <!-- optional -->

## Context and Problem Statement

[Describe the context and problem statement, e.g., in free form using two to three sentences. You may want to articulate the problem in form of a question.]

## Decision Drivers <!-- optional -->

* [driver 1, e.g., a force, facing concern, …]
* [driver 2, e.g., a force, facing concern, …]
* … <!-- numbers of drivers can vary -->

## Considered Options

* [option 1]
* [option 2]
* [option 3]
* … <!-- numbers of options can vary -->

## Decision Outcome

Chosen option: "[option 1]", because [justification. e.g., only option, which meets k.o. criterion decision driver | which resolves force force | … | comes out best (see below)].

### Positive Consequences <!-- optional -->

* [e.g., improvement of quality attribute satisfaction, follow-up decisions required, …]
* …

### Negative Consequences <!-- optional -->

* [e.g., compromising quality attribute, follow-up decisions required, …]
* …

## Pros and Cons of the Options <!-- optional -->

### [option 1]

[example | description | pointer to more information | …] <!-- optional -->

* Good, because [argument a]
* Good, because [argument b]
* Bad, because [argument c]
* … <!-- numbers of pros and cons can vary -->

### [option 2]

[example | description | pointer to more information | …] <!-- optional -->

* Good, because [argument a]
* Good, because [argument b]
* Bad, because [argument c]
* … <!-- numbers of pros and cons can vary -->

### [option 3]

[example | description | pointer to more information | …] <!-- optional -->

* Good, because [argument a]
* Good, because [argument b]
* Bad, because [argument c]
* … <!-- numbers of pros and cons can vary -->

## Links <!-- optional -->

* [Link type] [Link to ADR] <!-- example: Refined by [ADR-0005](0005-example.md) -->
* … <!-- numbers of links can vary -->
Loading