git-codereview: make git change handle GitHub PRs
Change-Id: Iab0417d14a29dce1a43f30f1f357d3f71dfdb51f
Reviewed-on: https://go-review.googlesource.com/c/review/+/279721
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
diff --git a/git-codereview/change.go b/git-codereview/change.go
index 6f76df1..5320d9d 100644
--- a/git-codereview/change.go
+++ b/git-codereview/change.go
@@ -103,16 +103,24 @@
}
func checkoutOrCreate(target string) {
- // If it's a valid Gerrit number, checkout the CL.
+ // If it's a valid Gerrit number CL or CL/PS or GitHub pull request number PR,
+ // checkout the CL or PR.
cl, ps, isCL := parseCL(target)
if isCL {
- if !haveGerrit() {
+ what := "CL"
+ if !haveGerrit() && haveGitHub() {
+ what = "PR"
+ if ps != "" {
+ dief("change PR syntax is NNN not NNN.PP")
+ }
+ }
+ if what == "CL" && !haveGerrit() {
dief("cannot change to a CL without gerrit")
}
if HasStagedChanges() || HasUnstagedChanges() {
- dief("cannot change to a CL with uncommitted work")
+ dief("cannot change to a %s with uncommitted work", what)
}
- checkoutCL(cl, ps)
+ checkoutCL(what, cl, ps)
return
}
@@ -176,8 +184,8 @@
}
// Checkout the patch set of the given CL. When patch set is empty, use the latest.
-func checkoutCL(cl, ps string) {
- if ps == "" {
+func checkoutCL(what, cl, ps string) {
+ if what == "CL" && ps == "" {
change, err := readGerritChange(cl + "?o=CURRENT_REVISION")
if err != nil {
dief("cannot change to CL %s: %v", cl, err)
@@ -189,28 +197,36 @@
ps = strconv.Itoa(rev.Number)
}
- var group string
- if len(cl) > 1 {
- group = cl[len(cl)-2:]
+ var ref string
+ if what == "CL" {
+ var group string
+ if len(cl) > 1 {
+ group = cl[len(cl)-2:]
+ } else {
+ group = "0" + cl
+ }
+ cl = fmt.Sprintf("%s/%s", cl, ps)
+ ref = fmt.Sprintf("refs/changes/%s/%s", group, cl)
} else {
- group = "0" + cl
+ ref = fmt.Sprintf("pull/%s/head", cl)
}
- ref := fmt.Sprintf("refs/changes/%s/%s/%s", group, cl, ps)
-
err := runErr("git", "fetch", "-q", "origin", ref)
if err != nil {
- dief("cannot change to CL %s/%s: %v", cl, ps, err)
+ dief("cannot change to %v %s: %v", what, cl, err)
}
err = runErr("git", "checkout", "-q", "FETCH_HEAD")
if err != nil {
- dief("cannot change to CL %s/%s: %v", cl, ps, err)
+ dief("cannot change to %s %s: %v", what, cl, err)
+ }
+ if *noRun {
+ return
}
subject, err := trimErr(cmdOutputErr("git", "log", "--format=%s", "-1"))
if err != nil {
- printf("changed to CL %s/%s.", cl, ps)
+ printf("changed to %s %s.", what, cl)
dief("cannot read change subject from git: %v", err)
}
- printf("changed to CL %s/%s.\n\t%s", cl, ps, subject)
+ printf("changed to %s %s.\n\t%s", what, cl, subject)
}
var parseCLRE = regexp.MustCompile(`^([0-9]+)(?:/([0-9]+))?$`)
diff --git a/git-codereview/change_test.go b/git-codereview/change_test.go
index d3b37b0..3e3ea30 100644
--- a/git-codereview/change_test.go
+++ b/git-codereview/change_test.go
@@ -160,6 +160,21 @@
checkChangeCL("100/1", "refs/changes/00/100/1", hash1)
checkChangeCL("100/2", "refs/changes/00/100/2", hash2)
checkChangeCL("100", "refs/changes/00/100/3", hash1)
+
+ // turn off gerrit, make it look like we are on GitHub
+ write(t, gt.server+"/codereview.cfg", "nothing: here", 0644)
+ trun(t, gt.server, "git", "add", "codereview.cfg")
+ trun(t, gt.server, "git", "commit", "-m", "new codereview.cfg on main")
+ testMain(t, "change", "main")
+ trun(t, gt.client, "git", "pull", "-r")
+ trun(t, gt.client, "git", "remote", "set-url", "origin", "https://github.com/google/not-a-project")
+
+ testMain(t, "change", "-n", "123")
+ testNoStdout(t)
+ testPrintedStderr(t,
+ "git fetch -q origin pull/123/head",
+ "git checkout -q FETCH_HEAD",
+ )
}
func TestChangeWithMessage(t *testing.T) {
diff --git a/git-codereview/config.go b/git-codereview/config.go
index d7a802e..4e0a565 100644
--- a/git-codereview/config.go
+++ b/git-codereview/config.go
@@ -75,6 +75,11 @@
return strings.HasSuffix(host, ".googlesource.com")
}
+func haveGitHub() bool {
+ origin := trim(cmdOutput("git", "config", "remote.origin.url"))
+ return strings.Contains(origin, "github.com")
+}
+
func parseConfig(raw string) (map[string]string, error) {
cfg := make(map[string]string)
for _, line := range nonBlankLines(raw) {
diff --git a/git-codereview/doc.go b/git-codereview/doc.go
index 4232869..a72b6db 100644
--- a/git-codereview/doc.go
+++ b/git-codereview/doc.go
@@ -154,10 +154,13 @@
option is only useful when creating commits (e.g. if there are unstaged
changes). If a commit already exists, it is overwritten. If -q is also
present, -q will be ignored.
-997As a special case, if branchname is a decimal CL number, such as 987, the change
+
+As a special case, if branchname is a decimal CL number, such as 987, the change
command downloads the latest patch set of that CL from the server and switches to it.
-A specific patch set can be requested by adding a decimal point: 987.2 for patch set 2
-of CL 987.
+A specific patch set P can be requested by adding /P: 987.2 for patch set 2 of CL 987.
+If the origin server is GitHub instead of Gerrit, then the number is
+treated a GitHub pull request number, and the change command downloads the latest
+version of that pull request. In this case, the /P suffix is disallowed.
Gofmt
