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

Align 3.1.1 with 3.0.4 #3953

Merged
merged 2 commits into from
Jul 23, 2024
Merged

Conversation

ralfhandl
Copy link
Contributor

@ralfhandl ralfhandl commented Jul 12, 2024

Editorial changes taken over from 3.0.4.

Additional changes here are to be synced back to 3.0.4 via #3966

@ralfhandl ralfhandl requested review from handrews and a team July 12, 2024 08:40
Comment on lines 2793 to 2799
<!-- 3.0.4 contains the following text: does it also apply to 3.1.1?
As such, the `discriminator` field MUST be a required field.
There are two ways to define the value of a discriminator for an inheriting instance.
- Use the schema name.
- Override the schema name by overriding the property with a new value. If a new value exists, this takes precedence over the schema name.
As such, inline schema definitions, which do not have a given id, *cannot* be used in polymorphism.
-->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this 3.0.4 text also apply to 3.1.1?

Copy link
Member

Choose a reason for hiding this comment

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

@ralfhandl this is separate from the changes I made - I don't think I ever touched this section (which is the section that #3951 is about). I can't recall the details of this MUST, but in #708 we apparently decided that the field would continue to be required.

Ideally, we would change this part in both versions, replacing "given id" with "component name" or "name within the Components Object":

As such, inline schema definitions, which do not have a given id, cannot be used in polymorphism.

This is confusing, because the "id" here is the name under the Components Object, not the $id keyword (which is supported in 3.1). I think I also avoided the phrase "inline schemas" when I did the other edits but can't remember right now- I'm less concerned about that.

Copy link
Contributor Author

@ralfhandl ralfhandl Jul 22, 2024

Choose a reason for hiding this comment

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

This text is present in 3.1.0 and apparently got lost during the refactoring, adding it back now.

The last sentence is not only confusing, it is also wrong as one can use $ref pointing to an "inline schema" anywhere in the OpenAPI document.

Adjusted the text accordingly and will "backport" this to 3.0.4 once we agree.

@@ -2790,8 +2788,15 @@ The OpenAPI Specification allows combining and extending model definitions using
`allOf` takes an array of object definitions that are validated *independently* but together compose a single object.

While composition offers model extensibility, it does not imply a hierarchy between the models.
To support polymorphism, the OpenAPI Specification adds the `discriminator` keyword.
To support polymorphism, the OpenAPI Specification adds the `discriminator` field.
Copy link
Contributor Author

@ralfhandl ralfhandl Jul 12, 2024

Choose a reason for hiding this comment

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

I cross-checked our predominant use of "keyword" and "field":

  • "keyword" if it is defined by JSON Schema
  • "field" if it is defined by OAS in one of the "Fixed Fields" sections

@ralfhandl ralfhandl marked this pull request as ready for review July 12, 2024 08:45
@ralfhandl ralfhandl added the editorial Wording and stylistic issues label Jul 12, 2024
@ralfhandl ralfhandl added this to the v3.1.1 milestone Jul 12, 2024
@miqui
Copy link
Contributor

miqui commented Jul 18, 2024

@handrews to check discriminator concern.

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

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

This looks good but we need to resolve the question about discriminator before we can approve

@lornajane lornajane requested a review from a team July 20, 2024 19:42
@ralfhandl ralfhandl requested a review from lornajane July 22, 2024 09:36
@ralfhandl
Copy link
Contributor Author

This looks good but we need to resolve the question about discriminator before we can approve

@lornajane Question resolved, please review

versions/3.1.1.md Show resolved Hide resolved
@ralfhandl ralfhandl merged commit ef319b2 into OAI:v3.1.1-dev Jul 23, 2024
1 check passed
@ralfhandl ralfhandl deleted the v3.1.1/sync-with-3.0.4 branch July 23, 2024 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Wording and stylistic issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants