-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
benchmark: fix install docs #10473
benchmark: fix install docs #10473
Conversation
tools/benchmark/README.md
Outdated
## Download | ||
To get `etcd` from the `master` branch via `go get`: | ||
```sh | ||
go get github.com/coreos/etcd/tools/benchmark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that 'go get' will perform download AND install.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, how about now?
c8dc45c
to
7b0b98b
Compare
Codecov Report
@@ Coverage Diff @@
## master #10473 +/- ##
==========================================
+ Coverage 71.57% 71.6% +0.03%
==========================================
Files 392 392
Lines 36518 36518
==========================================
+ Hits 26136 26147 +11
+ Misses 8552 8550 -2
+ Partials 1830 1821 -9
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding of the confusion in #10453 is the following. After user run 'go get ...' command, which performs download and install, he/she cannot find where the benchmark executable is located. If this is the case, I believe it is a standard setting issue regarding $GOPATH and $PATH, not specific to this benchmark tool. To make the doc more clear to new users, we need something similar to https://github.com/etcd-io/etcd/blob/master/Documentation/dl_build.md#build-the-latest-version
tools/benchmark/README.md
Outdated
```bash | ||
go get github.com/coreos/etcd/tools/benchmark | ||
## Download and install | ||
To get `etcd` from the `master` branch via `go get`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To get 'benchmark'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more comment. Then looks good to me. Thanks for fixing the doc.
tools/benchmark/README.md
Outdated
## Download and install | ||
To get `benchmark` from the `master` branch via `go get`: | ||
```sh | ||
$ go get github.com/coreos/etcd/tools/benchmark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I forgot to mention in my previous comment, can we also update the link to:
go get go.etcd.io/etcd/tools/benchmark
The etcd repo was moved to go.etcd.io (see #9965). Both links work because github is redirecting the old url to the new one. But I think it is better we update the link to reflect the new repo name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, get it.
|
lgtm |
#10463