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 naming convention of JSON Schema didn't matched with the spec #719

Merged

Conversation

co3k
Copy link
Contributor

@co3k co3k commented Aug 7, 2018

e.g. min_length should be converted as minLength in Swagger spec, but it wasn't.

please see also the schemaObject definition of OpenAPI v2 spec

This bug is introduced by #687 changes.

@codecov-io
Copy link

codecov-io commented Aug 7, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #719   +/-   ##
=======================================
  Coverage   56.16%   56.16%           
=======================================
  Files          30       30           
  Lines        3112     3112           
=======================================
  Hits         1748     1748           
  Misses       1193     1193           
  Partials      171      171
Impacted Files Coverage Δ
protoc-gen-swagger/genswagger/types.go 19.04% <ø> (ø) ⬆️

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 221ef34...2183528. Read the comment docs.

@johanbrandhorst
Copy link
Collaborator

Doh!

@johanbrandhorst
Copy link
Collaborator

Could you just include a link to the spec in the commit message and description as well? Good for future reference.

@co3k co3k force-pushed the fix-invalid-swagger-property-name branch from 59f6c06 to 2183528 Compare August 8, 2018 01:30
@ivucica ivucica merged commit bb916ca into grpc-ecosystem:master Aug 9, 2018
@ivucica
Copy link
Collaborator

ivucica commented Aug 9, 2018

Oops. I did not realize @johanbrandhorst requested a change. I only saw the change was approved. sigh

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.

5 participants