-
-
Notifications
You must be signed in to change notification settings - Fork 320
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
Introduce Adding examples to the OAS3 Media Type Object (Request / Response) #855
base: master
Are you sure you want to change the base?
Introduce Adding examples to the OAS3 Media Type Object (Request / Response) #855
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still need to look at it in more detail, but this looks promising
@@ -248,6 +248,9 @@ | |||
Strategies\ResponseFields\GetFromResponseFieldAttribute::class, | |||
Strategies\ResponseFields\GetFromResponseFieldTag::class, | |||
], | |||
'examples' => [ | |||
Strategies\Examples\UseExampleTag::class, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I feel like attributes are a more convenient way to specify multiple examples, so let's add support for that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shalvah let me know how you would like me to update the docs, should it be a new section under Eg: Documenting your API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say you can probably add it under the existing docs for requests and responses.
Been busy, will take a look at these sometime this week or next. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! This is a nice idea, and thanks for the screenshots. But my concern: How does this fit with the other response methods, especially when you throw in the scenario
feature? For instance, the response examples in your test cases are essentially the same as writing @response 200 <...>
.
When I think about it, the existing @response-
family of tags and annotations are already for specifying example responses, it's pretty much identical to these.
My current view:
- Rather than a generic
@example
tag, let's use the@response
tag and add a@request
tag, to align with the@response
tag. - Let's rather ensure the multiple example requests/responses are correctly written in the OpenAPI spec (I think that's what Allow multiple content types and schemas (via oneOf) according to OpenAPI 3.0 spec #792 is about).
- 'Group A' | ||
security: [] | ||
'/api/withExampleTagTypeRequest': | ||
get: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, doesn't seem like the contents match what should be generated here. I don't see any example requests.
FYI, I've merged #792, which I think implements some of this functionality. |
This PR introduces to create an entire request or response examples by leveraging existing OAS3 capabilities by allowing multiple examples with arbitrary names.
Having multiple examples will increase user experience as it allows consumers to see different ways that an API can be consumed / requested.
This is by default supported in most documentation tooling to help users pick which example they'd like to see. We could also expand examples into Objects and Parameters at a later stage.
Before:
After:
Scalar:
Rapidoc
Stoplight
Swagger
I can update the docs, if we can agree on this implementation.
Thank you!