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

Fix #1117: Support response examples. #1124

Merged

Conversation

jgiles
Copy link
Contributor

@jgiles jgiles commented Feb 1, 2020

Add support for examples in operation annotation responses. Merge
custom response data into the default 200 response.

Add support for examples in operation annotation responses. Merge
custom response data into the default 200 response.
@codecov-io
Copy link

codecov-io commented Feb 1, 2020

Codecov Report

Merging #1124 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1124      +/-   ##
==========================================
+ Coverage   53.63%   53.68%   +0.05%     
==========================================
  Files          42       42              
  Lines        4182     4187       +5     
==========================================
+ Hits         2243     2248       +5     
  Misses       1692     1692              
  Partials      247      247
Impacted Files Coverage Δ
protoc-gen-swagger/genswagger/template.go 56.37% <100%> (+0.2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 193538d...c1cf149. Read the comment docs.

Copy link
Collaborator

@johanbrandhorst johanbrandhorst 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 great to me, I had a question about the swagger.yaml change, but it's not a blocker. I was wondering if we could add a section to the docs explaining how a user can use this? Thanks for taking the time to implement this!

protoc-gen-swagger/genswagger/template.go Show resolved Hide resolved
examples/clients/abe/api/swagger.yaml Show resolved Hide resolved
@jgiles
Copy link
Contributor Author

jgiles commented Feb 1, 2020

Where in the documentation do you think this should live? I did a quick look-through, and wasn't sure where to put it.

For what it's worth, I generally look at a_bit_of_everything.proto when I want to see what I can do and how :-)

@johanbrandhorst
Copy link
Collaborator

Where in the documentation do you think this should live? I did a quick look-through, and wasn't sure where to put it.

I think we could do with a section on customising the swagger output. It could just be including some links to the big protofile. I appreciate your time is valuable so don't worry about writing too much.

For what it's worth, I generally look at a_bit_of_everything.proto when I want to see what I can do and how :-)

Yeah I often tell users this and it's not obvious enough to beginners. We should've had a link to this file for a long time already.

Copy link
Contributor Author

@jgiles jgiles left a comment

Choose a reason for hiding this comment

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

@johanbrandhorst I took a stab at some documentation improvements. They do not specifically reference this new feature, but they highlight that this kind of thing is possible and point at a_bit_of_everything.proto

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
docs/_docs/usage.md Show resolved Hide resolved
@johanbrandhorst johanbrandhorst merged commit e1a127c into grpc-ecosystem:master Feb 3, 2020
@johanbrandhorst
Copy link
Collaborator

Great work on this! Really appreciate the thoroughness and responsiveness.

@jgiles jgiles deleted the fix-1117-response-examples branch February 3, 2020 12:45
@jgiles
Copy link
Contributor Author

jgiles commented Feb 3, 2020

Thank you for being so responsive! You're doing great work as a maintainer in the ecosystem - we've seen that both in this project and in others (e.g. https://github.com/grpc-ecosystem/go-grpc-middleware ).

adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
)

* Fix grpc-ecosystem#1117: Support response examples.

Add support for examples in operation annotation responses. Merge
custom response data into the default 200 response.

* Regenerate files.

* Documentation improvements.
pull bot pushed a commit to BuildingRobotics/grpc-gateway that referenced this pull request Apr 29, 2020
)

* Fix grpc-ecosystem#1117: Support response examples.

Add support for examples in operation annotation responses. Merge
custom response data into the default 200 response.

* Regenerate files.

* Documentation improvements.
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.

4 participants