X Tutup
Skip to content

Commit 305cd29

Browse files
mislavmislav
committed
Fix pr checkout <owner>:<branch> when it matches the default branch
First, consolidate the functionality between `pr merge` and `pr checkout` that resolves the default branch name of the base repo. With an added bonus, the new approach avoids an API request when one isn't necessary. Then, ensure that checking out 3rd-party PRs will result in local branch name such as `<owner>/<branch>` when the head branch of the repository matches the default branch of the base repository. We already have had code in place to take care of this, but it only took effect in the `pr checkout <number>`-style invocation.
1 parent 6825944 commit 305cd29

File tree

7 files changed

+81
-145
lines changed

7 files changed

+81
-145
lines changed

‎api/queries_pr.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -432,9 +432,6 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu
432432
}
433433
headRepository {
434434
name
435-
defaultBranchRef {
436-
name
437-
}
438435
}
439436
isCrossRepository
440437
isDraft

‎api/queries_repo.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,18 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
114114
return initRepoHostname(&result.Repository, repo.RepoHost()), nil
115115
}
116116

117+
func RepoDefaultBranch(client *Client, repo ghrepo.Interface) (string, error) {
118+
if r, ok := repo.(*Repository); ok && r.DefaultBranchRef.Name != "" {
119+
return r.DefaultBranchRef.Name, nil
120+
}
121+
122+
r, err := GitHubRepo(client, repo)
123+
if err != nil {
124+
return "", err
125+
}
126+
return r.DefaultBranchRef.Name, nil
127+
}
128+
117129
// RepoParent finds out the parent repository of a fork
118130
func RepoParent(client *Client, repo ghrepo.Interface) (ghrepo.Interface, error) {
119131
var query struct {

‎command/pr.go‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -493,23 +493,21 @@ func prMerge(cmd *cobra.Command, args []string) error {
493493
fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title)
494494

495495
if deleteBranch {
496-
repo, err := api.GitHubRepo(apiClient, baseRepo)
497-
if err != nil {
498-
return err
499-
}
500-
501-
currentBranch, err := ctx.Branch()
502-
if err != nil {
503-
return err
504-
}
505-
506496
branchSwitchString := ""
507497

508498
if deleteLocalBranch && !crossRepoPR {
499+
currentBranch, err := ctx.Branch()
500+
if err != nil {
501+
return err
502+
}
503+
509504
var branchToSwitchTo string
510505
if currentBranch == pr.HeadRefName {
511-
branchToSwitchTo = repo.DefaultBranchRef.Name
512-
err = git.CheckoutBranch(repo.DefaultBranchRef.Name)
506+
branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo)
507+
if err != nil {
508+
return err
509+
}
510+
err = git.CheckoutBranch(branchToSwitchTo)
513511
if err != nil {
514512
return err
515513
}

‎command/pr_checkout.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/spf13/cobra"
1010

11+
"github.com/cli/cli/api"
1112
"github.com/cli/cli/git"
1213
"github.com/cli/cli/internal/ghrepo"
1314
"github.com/cli/cli/internal/run"
@@ -65,8 +66,13 @@ func prCheckout(cmd *cobra.Command, args []string) error {
6566
} else {
6667
// no git remote for PR head
6768

69+
defaultBranchName, err := api.RepoDefaultBranch(apiClient, baseRepo)
70+
if err != nil {
71+
return err
72+
}
73+
6874
// avoid naming the new branch the same as the default branch
69-
if newBranchName == pr.HeadRepository.DefaultBranchRef.Name {
75+
if newBranchName == defaultBranchName {
7076
newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName)
7177
}
7278

‎command/pr_checkout_test.go‎

Lines changed: 15 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
3333
"login": "hubot"
3434
},
3535
"headRepository": {
36-
"name": "REPO",
37-
"defaultBranchRef": {
38-
"name": "master"
39-
}
36+
"name": "REPO"
4037
},
4138
"isCrossRepository": false,
4239
"maintainerCanModify": false
@@ -84,10 +81,7 @@ func TestPRCheckout_urlArg(t *testing.T) {
8481
"login": "hubot"
8582
},
8683
"headRepository": {
87-
"name": "REPO",
88-
"defaultBranchRef": {
89-
"name": "master"
90-
}
84+
"name": "REPO"
9185
},
9286
"isCrossRepository": false,
9387
"maintainerCanModify": false
@@ -132,15 +126,17 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) {
132126
"login": "hubot"
133127
},
134128
"headRepository": {
135-
"name": "POE",
136-
"defaultBranchRef": {
137-
"name": "master"
138-
}
129+
"name": "POE"
139130
},
140131
"isCrossRepository": false,
141132
"maintainerCanModify": false
142133
} } } }
143134
`))
135+
http.Register(httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(`
136+
{ "data": { "repository": {
137+
"defaultBranchRef": {"name": "master"}
138+
} } }
139+
`))
144140

145141
ranCommands := [][]string{}
146142
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
@@ -195,10 +191,7 @@ func TestPRCheckout_branchArg(t *testing.T) {
195191
"login": "hubot"
196192
},
197193
"headRepository": {
198-
"name": "REPO",
199-
"defaultBranchRef": {
200-
"name": "master"
201-
}
194+
"name": "REPO"
202195
},
203196
"isCrossRepository": true,
204197
"maintainerCanModify": false }
@@ -245,10 +238,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {
245238
"login": "hubot"
246239
},
247240
"headRepository": {
248-
"name": "REPO",
249-
"defaultBranchRef": {
250-
"name": "master"
251-
}
241+
"name": "REPO"
252242
},
253243
"isCrossRepository": false,
254244
"maintainerCanModify": false
@@ -298,10 +288,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
298288
"login": "hubot"
299289
},
300290
"headRepository": {
301-
"name": "REPO",
302-
"defaultBranchRef": {
303-
"name": "master"
304-
}
291+
"name": "REPO"
305292
},
306293
"isCrossRepository": true,
307294
"maintainerCanModify": false
@@ -351,10 +338,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {
351338
"login": "hubot"
352339
},
353340
"headRepository": {
354-
"name": "REPO",
355-
"defaultBranchRef": {
356-
"name": "master"
357-
}
341+
"name": "REPO"
358342
},
359343
"isCrossRepository": true,
360344
"maintainerCanModify": false
@@ -404,10 +388,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
404388
"login": "hubot"
405389
},
406390
"headRepository": {
407-
"name": "REPO",
408-
"defaultBranchRef": {
409-
"name": "master"
410-
}
391+
"name": "REPO"
411392
},
412393
"isCrossRepository": true,
413394
"maintainerCanModify": false
@@ -455,10 +436,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
455436
"login": "hubot"
456437
},
457438
"headRepository": {
458-
"name": "REPO",
459-
"defaultBranchRef": {
460-
"name": "master"
461-
}
439+
"name": "REPO"
462440
},
463441
"isCrossRepository": true,
464442
"maintainerCanModify": false
@@ -506,10 +484,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
506484
"login": "hubot"
507485
},
508486
"headRepository": {
509-
"name": "REPO",
510-
"defaultBranchRef": {
511-
"name": "master"
512-
}
487+
"name": "REPO"
513488
},
514489
"isCrossRepository": true,
515490
"maintainerCanModify": true

0 commit comments

Comments
 (0)
X Tutup