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

Added comments to base_path to explain behavior #919

Merged
merged 6 commits into from
Jun 29, 2019

Conversation

nu11ptr
Copy link
Contributor

@nu11ptr nu11ptr commented Apr 5, 2019

Added comments to base_path to explain how it doesn't modify generated paths and has to be handled manually

Closes #918

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 this, however, could we start the comment out by saying what this does and give an example of where this might be useful, then add the note about how it doesn't change the generated paths in the http field? Thanks.

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.

Great, thanks for this!

@johanbrandhorst
Copy link
Collaborator

Ah, looks like you still need to regenerate the files with this new comment. There's a one liner in CONTRIBUTING.md that should work.

@johanbrandhorst
Copy link
Collaborator

Are you still interested in this PR? The CI build failure can be fixed by running two simple docker commands and committing the changes:

docker run -v $(pwd):/src/grpc-gateway --rm jfbrandhorst/grpc-gateway-build-env:1.12 \
    /bin/bash -c 'cd /src/grpc-gateway && \
        make realclean && \
        make examples SWAGGER_CODEGEN="${SWAGGER_CODEGEN}"'
docker run -itv $(pwd):/grpc-gateway -w /grpc-gateway --entrypoint /bin/bash --rm \
    l.gcr.io/google/bazel -c 'bazel run :gazelle; bazel run :buildifier'

go.mod Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #919   +/-   ##
=======================================
  Coverage   53.22%   53.22%           
=======================================
  Files          40       40           
  Lines        4004     4004           
=======================================
  Hits         2131     2131           
  Misses       1675     1675           
  Partials      198      198

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 d54b361...d384353. Read the comment docs.

@johanbrandhorst johanbrandhorst merged commit 740ef2e into grpc-ecosystem:master Jun 29, 2019
@johanbrandhorst
Copy link
Collaborator

Thanks for your contribution!

adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
* Add base_path docs to explain behavior

* Add to 'base_path' description

* Regenerated Go code from protobuf

* Revert go.mod to previous version

Closes grpc-ecosystem#918
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.

Documentation: 'base_path' Swagger attribute confuses users
4 participants