Skip to content

Commit 9a485dd

Browse files
mislavmislav
committed
💅 Cleanup local branch handling during pr checkout 
1 parent 294a029 commit 9a485dd

File tree

2 files changed

+28
-46
lines changed

2 files changed

+28
-46
lines changed

‎pkg/cmd/pr/checkout/checkout.go‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -167,26 +167,14 @@ func cmdsForExistingRemote(remote *context.Remote, pr *api.PullRequest, opts *Ch
167167
cmds = append(cmds, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)})
168168
}
169169
default:
170-
cmds = append(
171-
cmds,
172-
[]string{"git", "checkout", "-b", localBranch, "--no-track", remoteBranch},
173-
[]string{"git", "config", fmt.Sprintf("branch.%s.remote", localBranch), remote.Name},
174-
[]string{"git", "config", fmt.Sprintf("branch.%s.merge", pr.HeadRefName), "refs/heads/" + pr.HeadRefName},
175-
)
170+
cmds = append(cmds, []string{"git", "checkout", "-b", localBranch, "--track", remoteBranch})
176171
}
177172

178173
return cmds
179174
}
180175

181176
func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultBranch, protocol string, opts *CheckoutOptions) [][]string {
182177
var cmds [][]string
183-
184-
newBranchName := pr.HeadRefName
185-
// avoid naming the new branch the same as the default branch
186-
if newBranchName == defaultBranch {
187-
newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName)
188-
}
189-
190178
ref := fmt.Sprintf("refs/pull/%d/head", pr.Number)
191179

192180
if opts.Detach {
@@ -195,8 +183,16 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
195183
return cmds
196184
}
197185

186+
localBranch := pr.HeadRefName
187+
if opts.BranchName != "" {
188+
localBranch = opts.BranchName
189+
} else if pr.HeadRefName == defaultBranch {
190+
// avoid naming the new branch the same as the default branch
191+
localBranch = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, localBranch)
192+
}
193+
198194
currentBranch, _ := opts.Branch()
199-
if newBranchName == currentBranch {
195+
if localBranch == currentBranch {
200196
// PR head matches currently checked out branch
201197
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, ref})
202198
if opts.Force {
@@ -206,19 +202,14 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
206202
cmds = append(cmds, []string{"git", "merge", "--ff-only", "FETCH_HEAD"})
207203
}
208204
} else {
209-
// create a new branch
210205
if opts.Force {
211-
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, newBranchName), "--force"})
206+
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--force"})
212207
} else {
213208
// TODO: check if non-fast-forward and suggest to use `--force`
214-
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, newBranchName)})
209+
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch)})
215210
}
216211

217-
if opts.BranchName != "" {
218-
cmds = append(cmds, []string{"git", "checkout", "-b", opts.BranchName, "--track", newBranchName})
219-
} else {
220-
cmds = append(cmds, []string{"git", "checkout", newBranchName})
221-
}
212+
cmds = append(cmds, []string{"git", "checkout", localBranch})
222213
}
223214

224215
remote := baseURLOrName
@@ -228,9 +219,9 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
228219
remote = ghrepo.FormatRemoteURL(headRepo, protocol)
229220
mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName)
230221
}
231-
if missingMergeConfigForBranch(newBranchName) {
232-
cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote})
233-
cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef})
222+
if missingMergeConfigForBranch(localBranch) {
223+
cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.remote", localBranch), remote})
224+
cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.merge", localBranch), mergeRef})
234225
}
235226

236227
return cmds

‎pkg/cmd/pr/checkout/checkout_test.go‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,12 @@ func Test_checkoutRun(t *testing.T) {
101101
},
102102
},
103103
{
104-
name: "with local branch rename",
104+
name: "with local branch rename and existing git remote",
105105
opts: &CheckoutOptions{
106106
SelectorArg: "123",
107107
BranchName: "foobar",
108108
Finder: func() shared.PRFinder {
109-
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
110-
pr.MaintainerCanModify = true
111-
pr.HeadRepository = nil
109+
baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature")
112110
finder := shared.NewMockFinder("123", pr, baseRepo)
113111
return finder
114112
}(),
@@ -123,11 +121,9 @@ func Test_checkoutRun(t *testing.T) {
123121
"origin": "OWNER/REPO",
124122
},
125123
runStubs: func(cs *run.CommandStubber) {
126-
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
127-
cs.Register(`git config branch\.feature\.merge`, 1, "")
128-
cs.Register(`git checkout -b foobar --track feature`, 0, "")
129-
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
130-
cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "")
124+
cs.Register(`git show-ref --verify -- refs/heads/foobar`, 1, "")
125+
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
126+
cs.Register(`git checkout -b foobar --track origin/feature`, 0, "")
131127
},
132128
},
133129
{
@@ -138,7 +134,6 @@ func Test_checkoutRun(t *testing.T) {
138134
Finder: func() shared.PRFinder {
139135
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
140136
pr.MaintainerCanModify = true
141-
pr.HeadRepository = nil
142137
finder := shared.NewMockFinder("123", pr, baseRepo)
143138
return finder
144139
}(),
@@ -153,11 +148,11 @@ func Test_checkoutRun(t *testing.T) {
153148
"origin": "OWNER/REPO",
154149
},
155150
runStubs: func(cs *run.CommandStubber) {
156-
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
157-
cs.Register(`git config branch\.feature\.merge`, 1, "")
158-
cs.Register(`git checkout -b foobar --track feature`, 0, "")
159-
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
160-
cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "")
151+
cs.Register(`git config branch\.foobar\.merge`, 1, "")
152+
cs.Register(`git fetch origin refs/pull/123/head:foobar`, 0, "")
153+
cs.Register(`git checkout foobar`, 0, "")
154+
cs.Register(`git config branch\.foobar\.remote https://github.com/hubot/REPO.git`, 0, "")
155+
cs.Register(`git config branch\.foobar\.merge refs/heads/feature`, 0, "")
161156
},
162157
},
163158
}
@@ -272,9 +267,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
272267

273268
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
274269
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
275-
cs.Register(`git checkout -b feature --no-track origin/feature`, 0, "")
276-
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
277-
cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "")
270+
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")
278271

279272
output, err := runCommand(http, nil, "master", `123`)
280273
assert.NoError(t, err)
@@ -327,9 +320,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
327320

328321
cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature`, 0, "")
329322
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
330-
cs.Register(`git checkout -b feature --no-track robot-fork/feature`, 0, "")
331-
cs.Register(`git config branch\.feature\.remote robot-fork`, 0, "")
332-
cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "")
323+
cs.Register(`git checkout -b feature --track robot-fork/feature`, 0, "")
333324

334325
output, err := runCommand(http, remotes, "master", `123`)
335326
assert.NoError(t, err)

0 commit comments

Comments
 (0)