Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add CREF about ambiguous behaviour of additionalProperties #1203

Merged
merged 3 commits into from
Apr 14, 2022
Merged

Add CREF about ambiguous behaviour of additionalProperties #1203

merged 3 commits into from
Apr 14, 2022

Conversation

Relequestual
Copy link
Member

Fixes #1172
Must see new issue relating to the behaviour of annotation collection for resolving in the next draft.
Related to json-schema-org/community#57

@Relequestual Relequestual added this to the draft-patch for 2020-12 milestone Mar 30, 2022
@Relequestual Relequestual linked an issue Mar 30, 2022 that may be closed by this pull request
ambiguous behaviour of additionalProperties when invalid #1172
Closed
@Relequestual Relequestual mentioned this pull request Apr 1, 2022
Closed
2 tasks
karenetheridge
karenetheridge requested changes Apr 3, 2022
View reviewed changes
jsonschema-core.xml Outdated Show resolved Hide resolved
jsonschema-core.xml Outdated Show resolved Hide resolved
jsonschema-core.xml Outdated Show resolved Hide resolved
jsonschema-core.xml Outdated Show resolved Hide resolved
@Relequestual
Copy link
Member Author

I've removed some of the waffle from the CREF in the spec document as @jdesrosiers requested.
I've also added an Architectural Decision Record (ADR) for this overall decision to capture the reasoning behind it.
As you are named on the ADR (@karenetheridge @jdesrosiers @gregsdennis), it would be great to get your reviews.

Please feel free to make suggestion changes in your reviews if any additional thoughs come to mind or I've forgotten an option we considered.

karenetheridge
karenetheridge approved these changes Apr 8, 2022
View reviewed changes
Copy link
Member

@karenetheridge karenetheridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. Moving the decision content to an ADR is a good move as it allows more verbosity as well as linking to all the related issues where the discussions took place.

jdesrosiers
jdesrosiers approved these changes Apr 11, 2022
View reviewed changes
Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Creating an ADR for this was a great idea! If we had one for when the annotation based behavior was was introduced, we could have saved ourselves a lot of speculation about what the intention was.

adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md Outdated Show resolved Hide resolved
gregsdennis
gregsdennis approved these changes Apr 13, 2022
View reviewed changes
Copy link
Member

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor formatting issue. Good otherwise.

adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md Outdated Show resolved Hide resolved
@Relequestual
Add CREF about ambiguous behaviour of additionalProperties
30cd5fa 
Fixes json-schema-org#1172
Must see new issue relating to the behaviour of annotation collection for resolving in the next draft.
@Relequestual
Remove detail about ambiguity and reasoning. Link out to pending ADR
ef73bc8
@Relequestual
Add folder for architectural decision records (ADRs)
6b8542c 
Add first spec related ADR about the handling of additionalProperties ambiguity in 2019-09 and 2020-12 for the patch release by adding a CREF
@Relequestual Relequestual force-pushed the 1172-ambiguous-behaviour-additionalProperties-cref branch from dae2084 to 6b8542c Compare April 14, 2022 08:04
@Relequestual
Copy link
Member Author

Relequestual commented Apr 14, 2022 •
edited
Loading

Rebased to remove commits for spelling and grammar fixes.
Thank you all for your careful review and feedback.

@Relequestual Relequestual merged commit e8a6028 into json-schema-org:master Apr 14, 2022
@Relequestual Relequestual mentioned this pull request Apr 14, 2022
Closed
@handrews handrews mentioned this pull request Sep 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karenetheridge karenetheridge karenetheridge approved these changes

@jdesrosiers jdesrosiers jdesrosiers approved these changes

@gregsdennis gregsdennis gregsdennis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
draft-patch for 2020-12
Development

Successfully merging this pull request may close these issues.

ambiguous behaviour of additionalProperties when invalid
4 participants
@Relequestual @karenetheridge @jdesrosiers @gregsdennis