STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Allow deleting of local branch after merging cross repo PRs (#7709)

Browse source
This commit is contained in:
Armand Grillet 2023-07-25 18:03:33 +02:00 • committed by GitHub
parent 8079d18efd
commit fc685f92b6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 93 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

31
pkg/cmd/pr/merge/merge.go
View file

@ -199,7 +199,6 @@ type mergeContext struct {
autoMerge bool
crossRepoPR bool
deleteBranch bool
switchedToBranch string
mergeQueueRequired bool
}
@ -370,7 +369,7 @@ func (m *mergeContext) merge() error {
// Delete local branch if requested and if allowed.
func (m *mergeContext) deleteLocalBranch() error {
if m.crossRepoPR || m.autoMerge {
if m.autoMerge {
return nil
}
@ -395,6 +394,8 @@ func (m *mergeContext) deleteLocalBranch() error {
return err
}
switchedToBranch := ""
ctx := context.Background()
// branch the command was run on is the same as the pull request branch
@ -424,14 +425,19 @@ func (m *mergeContext) deleteLocalBranch() error {
_ = m.warnf(fmt.Sprintf("%s warning: not possible to fast-forward to: %q\n", m.cs.WarningIcon(), targetBranch))
}
m.switchedToBranch = targetBranch
switchedToBranch = targetBranch
}
if err := m.opts.GitClient.DeleteLocalBranch(ctx, m.pr.HeadRefName); err != nil {
return fmt.Errorf("failed to delete local branch %s: %w", m.cs.Cyan(m.pr.HeadRefName), err)
}
return nil
switchedStatement := ""
if switchedToBranch != "" {
switchedStatement = fmt.Sprintf(" and switched to branch %s", m.cs.Cyan(switchedToBranch))
}
return m.infof("%s Deleted local branch %s%s\n", m.cs.SuccessIconWithColor(m.cs.Red), m.cs.Cyan(m.pr.HeadRefName), switchedStatement)
}
// Delete the remote branch if requested and if allowed.
@ -451,11 +457,7 @@ func (m *mergeContext) deleteRemoteBranch() error {
}
}
branch := ""
if m.switchedToBranch != "" {
branch = fmt.Sprintf(" and switched to branch %s", m.cs.Cyan(m.switchedToBranch))
}
return m.infof("%s Deleted branch %s%s\n", m.cs.SuccessIconWithColor(m.cs.Red), m.cs.Cyan(m.pr.HeadRefName), branch)
return m.infof("%s Deleted remote branch %s\n", m.cs.SuccessIconWithColor(m.cs.Red), m.cs.Cyan(m.pr.HeadRefName))
}
// Add the Pull Request to a merge queue
@ -577,11 +579,16 @@ func mergeMethodSurvey(p shared.Prompt, baseRepo *api.Repository) (PullRequestMe
}
func deleteBranchSurvey(opts *MergeOptions, crossRepoPR, localBranchExists bool) (bool, error) {
if !crossRepoPR && !opts.IsDeleteBranchIndicated {
if !opts.IsDeleteBranchIndicated {
var message string
if opts.CanDeleteLocalBranch && localBranchExists {
message = "Delete the branch locally and on GitHub?"
} else {
if crossRepoPR {
message = "Delete the branch locally?"
} else {
message = "Delete the branch locally and on GitHub?"
}
} else if !crossRepoPR {
message = "Delete the branch on GitHub?"
}

82
pkg/cmd/pr/merge/merge_test.go
View file

@ -654,7 +654,8 @@ func TestPrMerge_deleteBranch(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
✓ Merged pull request #10 (Blueberries are a good fruit)
✓ Deleted branch blueberries and switched to branch main
✓ Deleted local branch blueberries and switched to branch main
✓ Deleted remote branch blueberries
`), output.Stderr())
}
@ -704,7 +705,56 @@ func TestPrMerge_deleteBranch_nonDefault(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
✓ Merged pull request #10 (Blueberries are a good fruit)
✓ Deleted branch blueberries and switched to branch fruit
✓ Deleted local branch blueberries and switched to branch fruit
✓ Deleted remote branch blueberries
`), output.Stderr())
}
func TestPrMerge_deleteBranch_onlyLocally(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
"",
&api.PullRequest{
ID: "PR_10",
Number: 10,
State: "OPEN",
Title: "Blueberries are a good fruit",
HeadRefName: "blueberries",
BaseRefName: "main",
MergeStateStatus: "CLEAN",
HeadRepositoryOwner: api.Owner{Login: "HEAD"}, // Not the same owner as the base repo
},
baseRepo("OWNER", "REPO", "main"),
)
http.Register(
httpmock.GraphQL(`mutation PullRequestMerge\b`),
httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
assert.Equal(t, "PR_10", input["pullRequestId"].(string))
assert.Equal(t, "MERGE", input["mergeMethod"].(string))
assert.NotContains(t, input, "commitHeadline")
}))
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git rev-parse --verify refs/heads/main`, 0, "")
cs.Register(`git checkout main`, 0, "")
cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "")
cs.Register(`git branch -D blueberries`, 0, "")
cs.Register(`git pull --ff-only`, 0, "")
output, err := runCommand(http, nil, "blueberries", true, `pr merge --merge --delete-branch`)
if err != nil {
t.Fatalf("Got unexpected error running `pr merge` %s", err)
}
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
✓ Merged pull request #10 (Blueberries are a good fruit)
✓ Deleted local branch blueberries and switched to branch main
`), output.Stderr())
}
@ -754,7 +804,8 @@ func TestPrMerge_deleteBranch_checkoutNewBranch(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
✓ Merged pull request #10 (Blueberries are a good fruit)
✓ Deleted branch blueberries and switched to branch fruit
✓ Deleted local branch blueberries and switched to branch fruit
✓ Deleted remote branch blueberries
`), output.Stderr())
}
@ -800,7 +851,8 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
✓ Merged pull request #10 (Blueberries are a good fruit)
✓ Deleted branch blueberries
✓ Deleted local branch blueberries
✓ Deleted remote branch blueberries
`), output.Stderr())
}
@ -1044,7 +1096,10 @@ func TestPrMerge_alreadyMerged(t *testing.T) {
output, err := runCommand(http, pm, "blueberries", true, "pr merge 4")
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "✓ Deleted branch blueberries and switched to branch main\n", output.Stderr())
assert.Equal(t, heredoc.Doc(`
✓ Deleted local branch blueberries and switched to branch main
✓ Deleted remote branch blueberries
`), output.Stderr())
}
func TestPrMerge_alreadyMerged_withMergeStrategy(t *testing.T) {
@ -1115,7 +1170,7 @@ func TestPrMerge_alreadyMerged_withMergeStrategy_TTY(t *testing.T) {
}
assert.Equal(t, "", output.String())
assert.Equal(t, "✓ Deleted branch \n", output.Stderr())
assert.Equal(t, "✓ Deleted local branch \n✓ Deleted remote branch \n", output.Stderr())
}
func TestPrMerge_alreadyMerged_withMergeStrategy_crossRepo(t *testing.T) {
@ -1139,7 +1194,17 @@ func TestPrMerge_alreadyMerged_withMergeStrategy_crossRepo(t *testing.T) {
cs.Register(`git rev-parse --verify refs/heads/`, 0, "")
output, err := runCommand(http, nil, "blueberries", true, "pr merge 4 --merge")
pm := &prompter.PrompterMock{
ConfirmFunc: func(p string, d bool) (bool, error) {
if p == "Pull request #4 was already merged. Delete the branch locally?" {
return d, nil
} else {
return false, prompter.NoSuchPromptErr(p)
}
},
}
output, err := runCommand(http, pm, "blueberries", true, "pr merge 4 --merge")
if err != nil {
t.Fatalf("Got unexpected error running `pr merge` %s", err)
}
@ -1282,7 +1347,8 @@ func TestPRMergeTTY_withDeleteBranch(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
✓ Merged pull request #3 (It was the best of times)
✓ Deleted branch blueberries and switched to branch main
✓ Deleted local branch blueberries and switched to branch main
✓ Deleted remote branch blueberries
`), output.Stderr())
}