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

Support extensions as nested annotations for extensible models #513

Merged
merged 5 commits into from
Mar 7, 2022

Conversation

MikeEdgar
Copy link
Member

Fixes #339
Fixes #387

@MikeEdgar MikeEdgar added this to the MP OpenAPI 3.1 milestone Feb 27, 2022
@MikeEdgar
Copy link
Member Author

We may want to add some additional TCK tests for some random selection of annotations.

@Azquelt
Copy link
Member

Azquelt commented Feb 28, 2022

Should we add the extensions array to the @Content annotation as well? As far as I can make out, it represents a key and value pair from the a content map. The value corresponds to a MediaType model object which is extensible.

@MikeEdgar
Copy link
Member Author

Should we add the extensions array to the @Content annotation as well?

Yes, thanks for catching it.

@Azquelt
Copy link
Member

Azquelt commented Feb 28, 2022

Should we add extensions to @APIResponses? I think that's allowed even though it's a repeatable container for @APIResponse as long as it has a default value and the APIResponses model object is extensible.

@Azquelt
Copy link
Member

Azquelt commented Feb 28, 2022

I'm unsure whether we should add it to @APISchemaResponse or not. I think that corresponds to a Response but it's also already a shorthand and users who want to add extensions could use the full @APIResponse instead.

@MikeEdgar
Copy link
Member Author

it's also already a shorthand and users who want to add extensions could use the full @APIResponse instead

That's my thinking as well.

Copy link
Member

@Azquelt Azquelt left a comment

Choose a reason for hiding this comment

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

I've finished my review and I'd be happy to approve once the comments above are addressed.

Copy link
Member

@Azquelt Azquelt left a comment

Choose a reason for hiding this comment

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

Changes look good but spotted one issue in the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extensions via Annotations Clarify Extensible name (with/without x-)
2 participants