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

Populate swagger method parameter description from message comments #692

Conversation

co3k
Copy link
Contributor

@co3k co3k commented Jul 3, 2018

No description provided.

@johanbrandhorst
Copy link
Collaborator

johanbrandhorst commented Jul 3, 2018

Hi @co3k, and thanks for your contribution! The title of this PR (and commit message) confuses me, could you reword it please? I don't understand what this does just from reading the title.

@co3k
Copy link
Contributor Author

co3k commented Jul 3, 2018

@johanbrandhorst
Oh, I'm sorry for confusing you. How about the following?

- Reflect descriptions of message to Swagger parameter representation automatically
+ Reflect comments of proto message as description of Swagger's Parameter Object representation implicitly

@johanbrandhorst
Copy link
Collaborator

Let me see if I understand correctly. Would this also be a correct description?

Populate swagger method parameter description from message comments

If so, I think that is clearer yet than your update ;).

@co3k co3k changed the title Reflect descriptions of message to Swagger parameter representation automatically Populate swagger method parameter description from message comments Jul 3, 2018
@co3k co3k force-pushed the reflect-message-comments-to-path-parameters branch from 200974d to 14eab3f Compare July 3, 2018 15:11
@co3k
Copy link
Contributor Author

co3k commented Jul 3, 2018

@johanbrandhorst Thank you for your good suggestion. I've updated the title of this pull request and the commit message.

@codecov-io
Copy link

codecov-io commented Jul 3, 2018

Codecov Report

Merging #692 into master will increase coverage by 0.01%.
The diff coverage is 64.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #692      +/-   ##
==========================================
+ Coverage   56.47%   56.48%   +0.01%     
==========================================
  Files          30       30              
  Lines        3005     3013       +8     
==========================================
+ Hits         1697     1702       +5     
- Misses       1145     1146       +1     
- Partials      163      165       +2
Impacted Files Coverage Δ
protoc-gen-swagger/genswagger/template.go 38.47% <64.28%> (+0.24%) ⬆️

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 39a18c6...afbe113. Read the comment docs.

if meth.GetClientStreaming() {
desc = "(streaming inputs)"
desc = desc + "(streaming inputs)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can just be desc += " (streaming inputs)" right? (Also notice the space before the first bracket)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Fixed.

@co3k co3k force-pushed the reflect-message-comments-to-path-parameters branch from 14eab3f to afbe113 Compare July 4, 2018 01:35
@johanbrandhorst johanbrandhorst merged commit e93c623 into grpc-ecosystem:master Jul 4, 2018
@co3k co3k deleted the reflect-message-comments-to-path-parameters branch July 4, 2018 09:08
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