Skip to content

Multiple openapi @dataExamples #224

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

msosnicki
Copy link

@msosnicki msosnicki commented Jun 13, 2025

This is how it looks like after the change

Screenshot from 2025-06-12 15-07-04

@@ -1,699 +1,711 @@
{
Copy link
Author

@msosnicki msosnicki Jun 13, 2025

Choose a reason for hiding this comment

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

This is unfortunate that this change is counted like that in git. VSCode is doing a better job visualizing the differences, by hiding unrelevant lines
Screenshot from 2025-06-13 08-46-35

@msosnicki msosnicki marked this pull request as ready for review June 13, 2025 06:58
@CLAassistant
Copy link

CLAassistant commented Jun 16, 2025

CLA assistant check
All committers have signed the CLA.

@msosnicki msosnicki requested a review from kubukoz June 16, 2025 11:38
Copy link
Member

@kubukoz kubukoz left a comment

Choose a reason for hiding this comment

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

mostly lgtm, just some nitpicks

@kubukoz
Copy link
Member

kubukoz commented Jun 16, 2025

This is how it looks like after the change

Screenshot from 2025-06-12 15-07-04

now that I'm looking at this up close, that doesn't seem right. It makes it seem Examples is a field...

from what I can see editor.swagger.io doesn't support OpenAPI 3.1:

image

The "old" example looks fine:

image

but with multiples, the best I can get is the same you got:

image

It seems confusing / misleading on Swagger UI's side :/

@msosnicki
Copy link
Author

Raised issue swagger-api/swagger-ui#10503

@kubukoz
Copy link
Member

kubukoz commented Jul 3, 2025

I assume Swagger will take its while to acknowledge and/or implement this. For now, let's hide it behind a configuration option (in addition to the OpenAPI 3.1 gate we already have) and move on

@msosnicki
Copy link
Author

I assume Swagger will take its while to acknowledge and/or implement this. For now, let's hide it behind a configuration option (in addition to the OpenAPI 3.1 gate we already have) and move on

We don't control OpenApiConfig unfortunately, it is coming from smithy-openapi module

@kubukoz
Copy link
Member

kubukoz commented Jul 8, 2025

I hoped there was a mechanism to put arbitrary properties in it :(

contribution idea for smithy-openapi?

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.

3 participants