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

cmd/go: accept @hash/branch in mod download #28042

Closed
wants to merge 1 commit into from

Conversation

marwan-at-work
Copy link
Contributor

@marwan-at-work marwan-at-work commented Oct 5, 2018

Go get in mod-enabled packages lets you do go get "pkg@" or "pkg@".
Go internally will switch the hash or branch into a pseudo version.
Go mod download should do the same. The bug lay in the fact that the disk cache
was not being written when Go converted the hash/branch into a pseudo version.

Fixes #27947

@marwan-at-work
Copy link
Contributor Author

marwan-at-work commented Oct 5, 2018

cc: @andybons gopherbot seems to be down as well as CLA bot etc. Would love some help porting this over to Gerrit.

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Oct 5, 2018
@gopherbot
Copy link
Contributor

This PR (HEAD: 0e6ac4f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/140257 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@marwan-at-work
Copy link
Contributor Author

or I just had to write a comment and everything works..

@gopherbot
Copy link
Contributor

Message from Gerrit User 13550:

Patch Set 1:

(3 comments)

Can't comment on the code itself as I'm not very used to it. Will leave that to Bryan.


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: e0227ae) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/140257 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 2: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 28030:

Patch Set 2:

Patch Set 1:

(3 comments)

Can't comment on the code itself as I'm not very used to it. Will leave that to Bryan.

I have updated the commit, but it's not being reflected on the Gerrit UI -- this is a CL that was opened through a GitHub PR. I don't mind closing and opening a new one now that I know how to open a CL directly without having to go through Github.


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5206:

Patch Set 2:

Can't comment on the code itself as I'm not very used to it. Will leave that to Bryan.

I have updated the commit, but it's not being reflected on the Gerrit UI -- this is a CL that was opened through a GitHub PR. I don't mind closing and opening a new one now that I know how to open a CL directly without having to go through Github.

You should be able to update the commit message on GitHub and have the change reflect in Gerrit, as described at the bottom of https://golang.org/wiki/CommitMessage#github-pull-requests .


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

@marwan-at-work marwan-at-work changed the title cmd/go: mod download should accept @hash/branch cmd/go: accept @hash/branch in mod download Nov 5, 2018
@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 3: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 4: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 4: Code-Review+1

(4 comments)

Thanks for the fix!


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

Go internally will switch the hash or branch into a pseudo version.
Go mod download should do the same. The bug lay in the fact that the disk cache
was not being written when Go converted the hash/branch into a pseudo version.

Fixes golang#27947

Change-Id: I55810a544ef4410f93c5b7ccbe7e2cad7c78b26f
@gopherbot
Copy link
Contributor

This PR (HEAD: 668634b) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/140257 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Marwan Sulaiman:

Patch Set 4:

(4 comments)

Updated CL per review


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 6: Run-TryBot+1 Code-Review+2


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 6:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=ee549110


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 6: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/140257.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Nov 16, 2018
Go get in mod-enabled packages lets you do go get "pkg@<hash>" or "pkg@<branch>".
Go internally will switch the hash or branch into a pseudo version.
Go mod download should do the same. The bug lay in the fact that the disk cache
was not being written when Go converted the hash/branch into a pseudo version.

Fixes #27947

Change-Id: I94c29a5c95f69ab18a9cd7a2ecade128047c5e36
GitHub-Last-Rev: 668634b
GitHub-Pull-Request: #28042
Reviewed-on: https://go-review.googlesource.com/c/140257
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/140257 has been merged.

@gopherbot gopherbot closed this Nov 16, 2018
bradfitz pushed a commit that referenced this pull request Nov 21, 2018
Go get in mod-enabled packages lets you do go get "pkg@<hash>" or "pkg@<branch>".
Go internally will switch the hash or branch into a pseudo version.
Go mod download should do the same. The bug lay in the fact that the disk cache
was not being written when Go converted the hash/branch into a pseudo version.

Fixes #27947

Change-Id: I94c29a5c95f69ab18a9cd7a2ecade128047c5e36
GitHub-Last-Rev: 668634b
GitHub-Pull-Request: #28042
Reviewed-on: https://go-review.googlesource.com/c/140257
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants