search

grpc-ecosystem / grpc-health-probe

A command-line tool to perform health-checks for gRPC applications in Kubernetes and elsewhere
Apache License 2.0
1.44k stars 188 forks source link

Updates binary name to comply with the go install command resulted name #169

Closed pantuza closed 5 months ago

pantuza commented 1 year ago

This Pull Request has a quite simple contribution.

When you install using go install github.com/grpc-ecosystem/grpc-health-probe@latest the binary name is actually grpc-health-probe instead of grpc_health_probe as described in the README.md file.

Thus, this pull request simply updates the binary name in the README.md documentation file.

What problem does it solve? Basically when a developer follows this documentation, they end up trying to run a command that does not exist in GOPATH. Since the binary name indeed does not exists, eventually, some developer can abort using the tool simply because it could not install it. Basically the binary is there, but the name is incorrect.

ahmetb commented 1 year ago

It solves one problem but incompatible everywhere else (e.g. docker images, tarball releases). Do you think addressing this discrepancy on line 47 where go install mentioned would help instead?

ahmetb commented 1 year ago

Another solution might be to rename the module in go.mod (to have _).

pantuza commented 1 year ago

Yes, indeed it makes more sense to me as well. I will make sure update this PR by changing the go.mod and we can check if does the trick.

pantuza commented 1 year ago

I have tested it by running go install locally and it works by installing a binary with the name grpc_health_probe under my $GOBIN path. Thank you for the suggestion. Please let me know if there is anything else we need to validated/test.

ahmetb commented 1 year ago

I have a feeling like this will break go-get due to repo name mismatch, if not, it'll break users who were installing via go-get. What do you think?

ahmetb commented 12 months ago

Sadly changing the module name will break too many users. https://sourcegraph.com/search?q=context:global+go+install+github.com/grpc-ecosystem/grpc-health&patternType=standard&sm=1&groupBy=repo I don't think this is a huge problem. We can just say "if installed via go install the binary name will be placed as grpc-health-probe under $GOBIN directory".

pantuza commented 11 months ago

Sadly changing the module name will break too many users. https://sourcegraph.com/search?q=context:global+go+install+github.com/grpc-ecosystem/grpc-health&patternType=standard&sm=1&groupBy=repo I don't think this is a huge problem. We can just say "if installed via go install the binary name will be placed as grpc-health-probe under $GOBIN directory".

Interesting. Yeah, I agree with you. I will make sure to add such entry on the README so you can review it, if that makes sense.

ahmetb commented 5 months ago

I think this is a little too much disruption to existing installations with little to no gain, sorry.