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.

Sign up for GitHub

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

Jump to bottom

Use 'git clone' instead of 'go get' for the build or developer's … #671

Closed
wants to merge 1 commit into from
Closed

Use 'git clone' instead of 'go get' for the build or developer's … #671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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\;
Copy link
Collaborator

@aalexand aalexand Nov 21, 2021 •
edited
Loading

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).

Copy link
Author

@chavey chavey Nov 21, 2021 •
edited
Loading

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.
"""

Copy link
Author

@chavey chavey Nov 21, 2021 •
edited
Loading

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.
"""

Copy link
Collaborator

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.

git clone [email protected]:google/pprof.git
```

To run the tests, do
Expand Down
6 changes: 4 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ Prerequisites:
- Graphviz: http://www.graphviz.org/
Optional, used to generate graphic visualizations of profiles

To build and install it, use the `go get` tool.
To build and install it, use `git clone`.

go get -u github.com/google/pprof
eval {'mkdir -p',cd}\ $GOPATH/src/github.com/google\;
git clone [email protected]:google/pprof.git
go install

Remember to set GOPATH to the directory where you want pprof to be
installed. The binary will be in `$GOPATH/bin` and the sources under
Expand Down