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

'type' should be 'integer' for int64 #810

Closed
wants to merge 1 commit into from

Conversation

koron
Copy link

@koron koron commented Nov 15, 2018

According to OpenAPI 2.0 spec, "type" should be "integer" for int64, not "string"
cf: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types

so current protoc-gen-swagger generates invalid swagger file when int64 is used.

This PR fix this problem, just replace "string" by "integer".

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@koron
Copy link
Author

koron commented Nov 15, 2018

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@koron koron force-pushed the integer-types branch 2 times, most recently from 050db02 to e75be2b Compare November 15, 2018 05:51
@codecov-io
Copy link

Codecov Report

Merging #810 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #810   +/-   ##
======================================
  Coverage    53.3%   53.3%           
======================================
  Files          30      30           
  Lines        3371    3371           
======================================
  Hits         1797    1797           
  Misses       1399    1399           
  Partials      175     175
Impacted Files Coverage Δ
protoc-gen-swagger/genswagger/template.go 38.52% <ø> (ø) ⬆️

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 1659831...e75be2b. Read the comment docs.

@johanbrandhorst
Copy link
Collaborator

This has been discussed before in #438, I don't think we will be changing this as it's fundamentally wrong when applied to JSON. See OAI/OpenAPI-Specification#704 for more discussion on this, but as the gRPC-Gateway works with JSON for swagger, we will not be changing this.

@koron
Copy link
Author

koron commented Nov 16, 2018

I see very well. Thank you for explaing it. 🙇

@koron koron deleted the integer-types branch November 16, 2018 01:04
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