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.

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

Make PathItem operation properties specific #2127

Merged
merged 1 commit into from
Mar 27, 2022

Conversation

axelcostaspena
Copy link
Contributor

PR for #2126.

Move the Operations properties definitions from patternProperties to the properties set, explicitly defining all of them. The goal is to get better coding tools completion - IntelliJ IDEA does - and to improve the spec quality with a purer spec schema usage.

Changes are 100% backwards compatible, all validating schemas will validate and all non validating schemas won't validate after merge. After merge, coding tools will get more information because of replacing a pattern with a defined literal set.

@handrews
Copy link
Member

handrews commented Feb 5, 2020

As the primary author of recent drafts of the JSON Schema spec, this is not an abuse of patternProperties. It's certainly valid to talk about what is most useful for tools, but it's a valid and useful technique as written.

@axelcostaspena
Copy link
Contributor Author

@handrews your opinion is welcome :)

Never mind, #2126 specifically states that its nº 1 goal is better tool support. Also, commit message first line is about specificity.

Copy link
Member

@MikeRalphson MikeRalphson left a comment

Choose a reason for hiding this comment

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

Aye from me.

Copy link
Member

@MikeRalphson MikeRalphson left a comment

Choose a reason for hiding this comment

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

Danger Will Robinson! This PR is against the OpenAPI.next branch. I do not believe any merges are scheduled from this branch. @webron please advise correct branch (and maybe we can remove OpenAPI.next?

@axelcostaspena
Copy link
Contributor Author

@MikeRalphson hmm sorry! Issue sems worse since the modification comes FROM OpenApi.next, which is unmerged into master. So let me solve it, I will create proper PR. But which work branch to choose? DEVELOPMENT.md:47 and README.md:24 seem to tell me to use 3.1.0-dev - non breaking change - instead of 3.0.3-dev - typos or clarifications - but none of that branch contain the v3 schema ¿?

@MikeRalphson
Copy link
Member

That guidance is for changes to the specification. The schema is non-normative, so I think master should be OK.

@handrews handrews added the Schema label Feb 7, 2020
Avoid misuse of `patternProperties` feature.
Fix OAI#2126
@axelcostaspena axelcostaspena changed the base branch from OpenAPI.next to master February 7, 2020 17:23
@axelcostaspena
Copy link
Contributor Author

@MikeRalphson PR source force updated
PR target updated.
Thank you for support on this.

Copy link
Member

@MikeRalphson MikeRalphson left a comment

Choose a reason for hiding this comment

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

Aye from me. Thanks for rebasing.

@webron webron merged commit 733c879 into OAI:main Mar 27, 2022
@karenetheridge
Copy link
Member

Should this also be merged into v3.1/schema*, as the same keyword usage exists there?

@webron
Copy link
Member

webron commented Mar 28, 2022

Wouldn't hurt...

karenetheridge added a commit to karenetheridge/OpenAPI-Specification that referenced this pull request Sep 11, 2022
karenetheridge added a commit to karenetheridge/OpenAPI-Specification that referenced this pull request Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants