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

x/tools/cmd/go-contrib-init: requires scratch repo to be set up as x/scratch #28332

Open
mvdan opened this issue Oct 23, 2018 · 2 comments
Open
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Oct 23, 2018

In previous Go Contributor Workshops, I had attendees run the following:

$ cd $(go env GOPATH)/src/golang.org/x
$ git clone https://go.googlesource.com/scratch
$ cd scratch
$ go-contrib-init -repo scratch
All good. Happy hacking!

However, that's tedious and not particularly portable. For example, it doesn't work with environments that use a multi-path GOPATH. So, since @kevinburke graciously set up a memorable domain name for it, I've tried to update the instructions to be the following:

$ go get -d goscratch.club
$ cd $(go list -f {{.Dir}} goscratch.club)
$ go-contrib-init -repo scratch
All good. Happy hacking!

However, that doesn't work, as go-contrib-init requires that all repos be checked out under golang.org/x:

$ pwd
/home/mvdan/go/land/src/goscratch.club
$ go-contrib-init -repo scratch
The repo you want to work on is currently not on your system.
Run "go get -d golang.org/x/scratch" to obtain this repo
then go to the directory "/home/mvdan/go/land/src/golang.org/x/scratch"

It's been decided that golang.org/x/scratch should not exist (#25921), so I think go-contrib-init should be taught that the scratch repo doesn't belong there.

I think it should accept any directory for it under GOPATH. Or, once we convert the subrepos to modules, the GOPATH directory check should be removed entirely.

/cc @dmitshur @bradfitz

@gopherbot gopherbot added this to the Unreleased milestone Oct 23, 2018
@dmitshur
Copy link
Contributor

dmitshur commented Oct 23, 2018

to be set up as x/tools/scratch

Did you mean "as x/scratch" in the title?

I think there's still value in pretending scratch would live at golang.org/x/scratch, if it were a real subrepo, because:

  1. The point of contributing to scratch is to learn how to contribute to other subrepos which do live in golang.org/x (other than dl and main repo).

  2. This would require adding a special case go-contrib-init, which adds complexity and increase risk of bugs in one path but not the others.

I agree that the experience for users should be good, so we should look for a way to improve the situation.

However, that's tedious and not particularly portable. For example, it doesn't work with environments that use a multi-path GOPATH.

It doesn't seem very tedious to me, compared to all the other steps that need to be taken while contributing.

However, it shouldn't fail if a person has a valid GOPATH list containing more than one directory in it.

Given they've already done go get -u golang.org/x/tools/cmd/go-contrib-init, we can be sure the golang.org/x directory exists in their GOPATH. So this should work:

$ cd $(go list -f '{{.Dir}}' -e golang.org/x)
$ git clone https://go.googlesource.com/scratch
$ cd scratch
$ go-contrib-init -repo scratch
All good. Happy hacking!

What do you think?

@ALTree ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 23, 2018
@mvdan mvdan changed the title x/tools/cmd/go-contrib-init: requires scratch repo to be set up as x/tools/scratch x/tools/cmd/go-contrib-init: requires scratch repo to be set up as x/scratch Oct 25, 2018
@mvdan
Copy link
Member Author

mvdan commented Oct 25, 2018

Did you mean "as x/scratch" in the title?

Yes.

The point of contributing to scratch is to learn how to contribute to other subrepos which do live in golang.org/x

That's sort of my point. For every other repo, one can go get -d golang.org/x/name, but we're teaching otherwise.

This would require adding a special case to go-contrib-init

The scratch repo already is a special case, so I'm not sure that this is a valid point.

I think there's still value in pretending scratch would live at golang.org/x/scratch.

I think this just ends up confusing newcomers. They try go get, and it fails, signaling that the project doesn't actually live there. They download the project elsewhere in GOPATH, and go-contrib-init fails.

What do you think?

That's a tiny bit better, but still leaves me with complex instructions for what should be trivial.

Ideally, once all of these repos are modules, we're just going to do:

$ git clone https://go.googlesource.com/scratch
$ cd scratch
$ go-contrib-init # knows what repo it's on because of go.mod

However, I understand that the x repos aren't going to get modules anytime soon, so I'm looking for a short-term improvement.

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants