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.

Sign up for GitHub

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

Jump to bottom

protoc-gen-openapiv2: Add openapiv2_tag support #1866

Merged
merged 6 commits into from
Dec 9, 2020
Merged

protoc-gen-openapiv2: Add openapiv2_tag support #1866

merged 6 commits into from
Dec 9, 2020

Conversation

rcgoodfellow
Copy link
Contributor

References to other Issues or PRs

Fixes #690

Brief description of what is fixed or changed

Added a renderServiceTag method to pick up openapiv2_tag options. The change is pretty surgical aside from the fact that the return type for renderServices was changed to propagate discovered tags back to the caller. So a small pile of return statements had to be updated.

Other comments

Thanks to the grpc-gateway team, it's a really great tool.

Ryan Goodfellow added 2 commits December 7, 2020 22:35
protoc-gen-openapiv2: support for openapiv2_tag
9cdf8c0 
Fixes grpc-ecosystem#690

Signed-off-by: Ryan Goodfellow <rgoodfel@isi.edu>
more consistent with rest of codebase
cdc19ea 
Signed-off-by: Ryan Goodfellow <rgoodfel@isi.edu>
@google-cla google-cla bot added the cla: yes label Dec 8, 2020
@rcgoodfellow rcgoodfellow mentioned this pull request Dec 8, 2020
Closed
update example specs, revert protoc version change
0f4d05c 
Signed-off-by: Ryan Goodfellow <rgoodfel@isi.edu>
johanbrandhorst
johanbrandhorst requested changes Dec 8, 2020
View reviewed changes
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.

Thanks for taking this on! It doesn't look too bad. Some minor cosmetic suggestions and a question around the use of renderServices is all.

examples/internal/proto/examplepb/echo_service.swagger.json Outdated Show resolved Hide resolved
protoc-gen-openapiv2/internal/genopenapi/template.go Outdated Show resolved Hide resolved
protoc-gen-openapiv2/internal/genopenapi/template.go Outdated Show resolved Hide resolved
protoc-gen-openapiv2/internal/genopenapi/template.go Outdated Show resolved Hide resolved
protoc-gen-openapiv2/internal/genopenapi/types.go Outdated Show resolved Hide resolved
Address review comments
d2c1e6c 
- Factor service tag rendering out of service rendering function
- Add omitempty tags where applicable

Signed-off-by: Ryan Goodfellow <rgoodfel@isi.edu>
@rcgoodfellow
Copy link
Contributor Author

Failed CI task appears to be an issue with the fuzzit service

2020/12/08 17:08:33 Get https://app.fuzzit.dev/getStorageLinkV3?path=orgs%2Fgrpc-gateway%2Ftargets%2Fparse-http-rule%2Fcorpus.tar.gz&api_key=&action=read: x509: certificate is valid for c.sni.fastly.net, not app.fuzzit.dev

@rcgoodfellow
Copy link
Contributor Author

Thanks for the feedback and quick response @johanbrandhorst. I believe I've addressed the comments.

Ryan Goodfellow added 2 commits December 8, 2020 09:32
remove extraneous newline
40b0593 
Signed-off-by: Ryan Goodfellow <rgoodfel@isi.edu>
renderServices call to previous form
7377de7 
Signed-off-by: Ryan Goodfellow <rgoodfel@isi.edu>
johanbrandhorst
johanbrandhorst approved these changes Dec 9, 2020
View reviewed changes
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.

LGTM

@johanbrandhorst johanbrandhorst merged commit 82c9623 into grpc-ecosystem:master Dec 9, 2020
@johanbrandhorst
Copy link
Collaborator

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johanbrandhorst johanbrandhorst johanbrandhorst approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support openapiv2_tag option in swagger generator
2 participants
@rcgoodfellow @johanbrandhorst