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

Swagger 2.4.8 #1058

Merged
merged 5 commits into from Oct 4, 2019
Merged

Conversation

zachgersh
Copy link
Contributor

@zachgersh zachgersh commented Oct 3, 2019

  • had to update the makefile as the generated
    file names have all moved around a little bit
  • moved bazel forward to use go 1.13.1 as well
  • test that was using a deep reflect to do equality
    has changed to be per struct field equality (deep
    reflect no longer works with the enum types).
  • swagger 2.4.8 actually removed the need for resty
    as they use the standard lib for http requests.

Biggest bit - you cannot generate both an rpc and a regular service in the same directory at this version. Why? In our example they both use an optional type which used to be simple a string but now swagger-codegen creates a struct. The struct has the same name for both versions and you get a type redeclaration (sad).

Went and had a look at swagger-codegens repo for any open issues around this and saw 0. I don't think users actually do this so it seemed alright to remove it from our test generation.

Zach Gershman added 2 commits October 2, 2019 10:11
- had to update the makefile as the generated
file names have all moved around a little bit
- moved bazel forward to use go 1.13.1 as well
- test that was using a deep reflect to do equality
has changed to be per struct field equality (deep
reflect no longer works with the enum types).
@johanbrandhorst
Copy link
Collaborator

johanbrandhorst commented Oct 3, 2019

swagger 2.4.8 actually removed the need for resty as they use the standard lib for http requests.

This is the best news I've heard in weeks.

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, lets get it in ASAP.

.circleci/Dockerfile Outdated Show resolved Hide resolved
examples/integration/integration_test.go Show resolved Hide resolved
@zachgersh
Copy link
Contributor Author

Would need one more dockerfile rebuild here.

@codecov-io
Copy link

codecov-io commented Oct 4, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1058   +/-   ##
=======================================
  Coverage   53.84%   53.84%           
=======================================
  Files          41       41           
  Lines        4147     4147           
=======================================
  Hits         2233     2233           
  Misses       1670     1670           
  Partials      244      244

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 28a1cae...40159c7. Read the comment docs.

Co-Authored-By: Johan Brandhorst <johan.brandhorst@gmail.com>
@johanbrandhorst
Copy link
Collaborator

Lets merge this, I'll do a rebuild of the image on master.

@johanbrandhorst johanbrandhorst merged commit c677e41 into grpc-ecosystem:master Oct 4, 2019
@zachgersh zachgersh deleted the swagger-2.4.8 branch October 4, 2019 15:09
adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
* generate files with swagger-codegen 2.4.8

* swagger-codegen 2.4.8 is now the base requirement

- had to update the makefile as the generated
file names have all moved around a little bit
- moved bazel forward to use go 1.13.1 as well
- test that was using a deep reflect to do equality
has changed to be per struct field equality (deep
reflect no longer works with the enum types).

* remove need for env var

* adds -e plus refactors tools to top

* Update .circleci/Dockerfile

Co-Authored-By: Johan Brandhorst <johan.brandhorst@gmail.com>
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.

None yet

4 participants