💅 Cleanup local branch handling during pr checkout

This commit is contained in:
Mislav Marohnić 2021-08-05 20:42:52 +02:00
parent 294a029e70
commit 9a485ddfa2
2 changed files with 28 additions and 46 deletions

View file

@ -167,12 +167,7 @@ func cmdsForExistingRemote(remote *context.Remote, pr *api.PullRequest, opts *Ch
cmds = append(cmds, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)})
}
default:
cmds = append(
cmds,
[]string{"git", "checkout", "-b", localBranch, "--no-track", remoteBranch},
[]string{"git", "config", fmt.Sprintf("branch.%s.remote", localBranch), remote.Name},
[]string{"git", "config", fmt.Sprintf("branch.%s.merge", pr.HeadRefName), "refs/heads/" + pr.HeadRefName},
)
cmds = append(cmds, []string{"git", "checkout", "-b", localBranch, "--track", remoteBranch})
}
return cmds
@ -180,13 +175,6 @@ func cmdsForExistingRemote(remote *context.Remote, pr *api.PullRequest, opts *Ch
func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultBranch, protocol string, opts *CheckoutOptions) [][]string {
var cmds [][]string
newBranchName := pr.HeadRefName
// avoid naming the new branch the same as the default branch
if newBranchName == defaultBranch {
newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName)
}
ref := fmt.Sprintf("refs/pull/%d/head", pr.Number)
if opts.Detach {
@ -195,8 +183,16 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
return cmds
}
localBranch := pr.HeadRefName
if opts.BranchName != "" {
localBranch = opts.BranchName
} else if pr.HeadRefName == defaultBranch {
// avoid naming the new branch the same as the default branch
localBranch = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, localBranch)
}
currentBranch, _ := opts.Branch()
if newBranchName == currentBranch {
if localBranch == currentBranch {
// PR head matches currently checked out branch
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, ref})
if opts.Force {
@ -206,19 +202,14 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
cmds = append(cmds, []string{"git", "merge", "--ff-only", "FETCH_HEAD"})
}
} else {
// create a new branch
if opts.Force {
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, newBranchName), "--force"})
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--force"})
} else {
// TODO: check if non-fast-forward and suggest to use `--force`
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, newBranchName)})
cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch)})
}
if opts.BranchName != "" {
cmds = append(cmds, []string{"git", "checkout", "-b", opts.BranchName, "--track", newBranchName})
} else {
cmds = append(cmds, []string{"git", "checkout", newBranchName})
}
cmds = append(cmds, []string{"git", "checkout", localBranch})
}
remote := baseURLOrName
@ -228,9 +219,9 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
remote = ghrepo.FormatRemoteURL(headRepo, protocol)
mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName)
}
if missingMergeConfigForBranch(newBranchName) {
cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote})
cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef})
if missingMergeConfigForBranch(localBranch) {
cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.remote", localBranch), remote})
cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.merge", localBranch), mergeRef})
}
return cmds

View file

@ -101,14 +101,12 @@ func Test_checkoutRun(t *testing.T) {
},
},
{
name: "with local branch rename",
name: "with local branch rename and existing git remote",
opts: &CheckoutOptions{
SelectorArg: "123",
BranchName: "foobar",
Finder: func() shared.PRFinder {
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
pr.MaintainerCanModify = true
pr.HeadRepository = nil
baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature")
finder := shared.NewMockFinder("123", pr, baseRepo)
return finder
}(),
@ -123,11 +121,9 @@ func Test_checkoutRun(t *testing.T) {
"origin": "OWNER/REPO",
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout -b foobar --track feature`, 0, "")
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/foobar`, 1, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git checkout -b foobar --track origin/feature`, 0, "")
},
},
{
@ -138,7 +134,6 @@ func Test_checkoutRun(t *testing.T) {
Finder: func() shared.PRFinder {
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
pr.MaintainerCanModify = true
pr.HeadRepository = nil
finder := shared.NewMockFinder("123", pr, baseRepo)
return finder
}(),
@ -153,11 +148,11 @@ func Test_checkoutRun(t *testing.T) {
"origin": "OWNER/REPO",
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout -b foobar --track feature`, 0, "")
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "")
cs.Register(`git config branch\.foobar\.merge`, 1, "")
cs.Register(`git fetch origin refs/pull/123/head:foobar`, 0, "")
cs.Register(`git checkout foobar`, 0, "")
cs.Register(`git config branch\.foobar\.remote https://github.com/hubot/REPO.git`, 0, "")
cs.Register(`git config branch\.foobar\.merge refs/heads/feature`, 0, "")
},
},
}
@ -272,9 +267,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git checkout -b feature --no-track origin/feature`, 0, "")
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "")
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")
output, err := runCommand(http, nil, "master", `123`)
assert.NoError(t, err)
@ -327,9 +320,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git checkout -b feature --no-track robot-fork/feature`, 0, "")
cs.Register(`git config branch\.feature\.remote robot-fork`, 0, "")
cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "")
cs.Register(`git checkout -b feature --track robot-fork/feature`, 0, "")
output, err := runCommand(http, remotes, "master", `123`)
assert.NoError(t, err)