-
Notifications
You must be signed in to change notification settings - Fork 605
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
Use 'git clone' instead of 'go get' for the build or developer's … #671
Conversation
…low documentation. As golang released their new version 1.17.1 go get is not supported anymore. golang/go#31529 (comment) suggests using 'git clone' rather than 'go get'.
@@ -45,7 +45,8 @@ automatically set in all console sessions. | |||
To get the source code, run | |||
|
|||
``` | |||
go get github.com/google/pprof | |||
eval {'mkdir -p',cd}\ $GOPATH/src/github.com/google\; |
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.
"eval" seems scary, as well as creating a directory for people. This will also fail if $GOPATH is not set since this will try to create a directory at the root ("/src/github.com/google").
It also creates impression that the source directory has to be under $GOPATH, but $GOPATH is considered largely deprecated since Go 1.13 (I believe its only intended usage is being able to override where the module cache is stored, but that should be rare / not needed).
So, I think I would instead use
cd /tmp
git clone [email protected]:google/pprof.git
cd pprof
and change cd $GOPATH/src/github.com/google/pprof
to cd /tmp/pprof
in other commands below. This should make it clear that the checkout directory can be in any location and people who read the instruction will override it with their favorite path (e.g. I use ~/src/pprof
).
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.
do we want to replace:
"""
Make sure GOPATH
is set in your current shell. The common way is to have
something like export GOPATH=$HOME/gocode
in your .bashrc
file so that it's
automatically set in all console sessions.
"""
with
"""
Make sure to use a GoLang version 1.13 or above.
"""
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.
Do we want to keep:
"""
When you wish to work with your own fork of the source (which is required to be able to create a pull request), you'll want to get your fork repo as another Git remote in the same github.com/google/pprof
directory. Otherwise, if you'll go get
your fork directly, you'll be getting errors like use of internal package not allowed
when running tests. To set up the remote do something like
cd $GOPATH/src/github.com/google/pprof
git remote add aalexand [email protected]:aalexand/pprof.git
git fetch aalexand
git checkout -b my-new-feature
# hack hack hack
go test -v ./...
git commit -a -m "Add new feature."
git push aalexand
where aalexand
is your GitHub user ID. Then proceed to the GitHub UI to send a
code review.
"""
We could replace all the above by:
"""
To make changes to the code:
cd /tmp/pprof
git fetch origin
git checkout -b my-new-feature
# hack hack hack
go test -v ./...
git commit -a -m "Add a new feature."
git push origin
Then proceed to the GitHub UI to send a
code review.
"""
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.
We don't need to say "Make sure to use a GoLang version 1.13 or above" because we already reference https://golang.org/doc/devel/release#policy which as of now means that pprof only supports Go 1.17 and Go 1.16.
Use 'git clone' instead of 'go get' for the build or developer's workflow documentation.
As golang released their new version 1.17.1 go get is not supported
anymore. golang/go#31529 (comment)
suggests using 'git clone' rather than 'go get'.