Restore backwards compatibility in scripted pr merge

This commit is contained in:
Mislav Marohnić 2020-10-16 18:27:12 +00:00 committed by GitHub
parent 7d317a81be
commit 89529674f9
2 changed files with 130 additions and 94 deletions

View file

@ -33,7 +33,6 @@ type MergeOptions struct {
DeleteLocalBranch bool
MergeMethod api.PullRequestMergeMethod
InteractiveMode bool
ConfirmSubmit bool
}
func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Command {
@ -108,7 +107,6 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm
cmd.Flags().BoolVarP(&flagMerge, "merge", "m", false, "Merge the commits with the base branch")
cmd.Flags().BoolVarP(&flagRebase, "rebase", "r", false, "Rebase the commits onto the base branch")
cmd.Flags().BoolVarP(&flagSquash, "squash", "s", false, "Squash the commits into one commit and merge it into the base branch")
cmd.Flags().BoolVarP(&opts.ConfirmSubmit, "confirm", "y", false, "Confirm Merging the PR (default: false)")
return cmd
}
@ -142,101 +140,94 @@ func mergeRun(opts *MergeOptions) error {
if opts.InteractiveMode {
mergeMethod, deleteBranch, err = prInteractiveMerge(opts.DeleteLocalBranch, crossRepoPR)
if err != nil {
return nil
}
}
shouldSubmitFromFlag := opts.ConfirmSubmit
if !shouldSubmitFromFlag {
fmt.Println(opts.ConfirmSubmit)
err := prompt.Confirm("Submit? ", &shouldSubmitFromFlag)
if err != nil {
if errors.Is(err, cancelError) {
fmt.Fprintln(opts.IO.ErrOut, "Cancelled.")
return cmdutil.SilentError
}
return err
}
}
if shouldSubmitFromFlag {
var action string
if mergeMethod == api.PullRequestMergeMethodRebase {
action = "Rebased and merged"
err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodRebase)
} else if mergeMethod == api.PullRequestMergeMethodSquash {
action = "Squashed and merged"
err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodSquash)
} else if mergeMethod == api.PullRequestMergeMethodMerge {
action = "Merged"
err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodMerge)
} else {
err = fmt.Errorf("unknown merge method (%d) used", mergeMethod)
return err
}
var action string
if mergeMethod == api.PullRequestMergeMethodRebase {
action = "Rebased and merged"
err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodRebase)
} else if mergeMethod == api.PullRequestMergeMethodSquash {
action = "Squashed and merged"
err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodSquash)
} else if mergeMethod == api.PullRequestMergeMethodMerge {
action = "Merged"
err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodMerge)
} else {
err = fmt.Errorf("unknown merge method (%d) used", mergeMethod)
return err
}
if err != nil {
return fmt.Errorf("API call failed: %w", err)
}
if err != nil {
return fmt.Errorf("API call failed: %w", err)
}
isTerminal := opts.IO.IsStdoutTTY()
isTerminal := opts.IO.IsStdoutTTY()
if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title)
}
if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title)
}
if deleteBranch {
branchSwitchString := ""
if deleteBranch {
branchSwitchString := ""
if opts.DeleteLocalBranch && !crossRepoPR {
currentBranch, err := opts.Branch()
if opts.DeleteLocalBranch && !crossRepoPR {
currentBranch, err := opts.Branch()
if err != nil {
return err
}
var branchToSwitchTo string
if currentBranch == pr.HeadRefName {
branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo)
if err != nil {
return err
}
var branchToSwitchTo string
if currentBranch == pr.HeadRefName {
branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo)
if err != nil {
return err
}
err = git.CheckoutBranch(branchToSwitchTo)
if err != nil {
return err
}
}
localBranchExists := git.HasLocalBranch(pr.HeadRefName)
if localBranchExists {
err = git.DeleteLocalBranch(pr.HeadRefName)
if err != nil {
err = fmt.Errorf("failed to delete local branch %s: %w", utils.Cyan(pr.HeadRefName), err)
return err
}
}
if branchToSwitchTo != "" {
branchSwitchString = fmt.Sprintf(" and switched to branch %s", utils.Cyan(branchToSwitchTo))
}
}
if !crossRepoPR {
err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName)
var httpErr api.HTTPError
// The ref might have already been deleted by GitHub
if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) {
err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err)
err = git.CheckoutBranch(branchToSwitchTo)
if err != nil {
return err
}
}
if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString)
localBranchExists := git.HasLocalBranch(pr.HeadRefName)
if localBranchExists {
err = git.DeleteLocalBranch(pr.HeadRefName)
if err != nil {
err = fmt.Errorf("failed to delete local branch %s: %w", utils.Cyan(pr.HeadRefName), err)
return err
}
}
if branchToSwitchTo != "" {
branchSwitchString = fmt.Sprintf(" and switched to branch %s", utils.Cyan(branchToSwitchTo))
}
}
} else {
fmt.Println("Discarding")
if !crossRepoPR {
err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName)
var httpErr api.HTTPError
// The ref might have already been deleted by GitHub
if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) {
err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err)
return err
}
}
if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString)
}
}
return nil
}
var cancelError = errors.New("cancelError")
func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullRequestMergeMethod, bool, error) {
mergeMethodQuestion := &survey.Question{
Name: "mergeMethod",
@ -267,15 +258,27 @@ func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullReque
qs = append(qs, deleteBranchQuestion)
}
qs = append(qs, &survey.Question{
Name: "isConfirmed",
Prompt: &survey.Confirm{
Message: "Submit?",
Default: false,
},
})
answers := struct {
MergeMethod int
DeleteBranch bool
IsConfirmed bool
}{}
err := prompt.SurveyAsk(qs, &answers)
if err != nil {
return 0, false, fmt.Errorf("could not prompt: %w", err)
}
if !answers.IsConfirmed {
return 0, false, cancelError
}
var mergeMethod api.PullRequestMergeMethod
switch answers.MergeMethod {

View file

@ -2,6 +2,7 @@ package merge
import (
"bytes"
"errors"
"io/ioutil"
"net/http"
"regexp"
@ -198,8 +199,6 @@ func TestPrMerge(t *testing.T) {
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
prompt.StubConfirm(true)
cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge)
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
cs.Stub("") // git symbolic-ref --quiet --short HEAD
@ -246,8 +245,6 @@ func TestPrMerge_nontty(t *testing.T) {
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
prompt.StubConfirm(true)
cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge)
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
cs.Stub("") // git symbolic-ref --quiet --short HEAD
@ -291,8 +288,6 @@ func TestPrMerge_withRepoFlag(t *testing.T) {
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
prompt.StubConfirm(true)
output, err := runCommand(http, "master", true, "pr merge 1 --merge -R OWNER/REPO")
if err != nil {
t.Fatalf("error running command `pr merge`: %v", err)
@ -328,8 +323,6 @@ func TestPrMerge_deleteBranch(t *testing.T) {
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
prompt.StubConfirm(true)
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
cs.Stub("") // git checkout master
cs.Stub("") // git rev-parse --verify blueberries`
@ -365,8 +358,6 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
prompt.StubConfirm(true)
// We don't expect the default branch to be checked out, just that blueberries is deleted
cs.Stub("") // git rev-parse --verify blueberries
cs.Stub("") // git branch -d blueberries
@ -401,8 +392,6 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) {
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
prompt.StubConfirm(true)
cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge)
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
cs.Stub("") // git symbolic-ref --quiet --short HEAD
@ -449,8 +438,6 @@ func TestPrMerge_rebase(t *testing.T) {
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
prompt.StubConfirm(true)
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
cs.Stub("") // git symbolic-ref --quiet --short HEAD
cs.Stub("") // git checkout master
@ -496,8 +483,6 @@ func TestPrMerge_squash(t *testing.T) {
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
prompt.StubConfirm(true)
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
cs.Stub("") // git symbolic-ref --quiet --short HEAD
cs.Stub("") // git checkout master
@ -524,8 +509,6 @@ func TestPrMerge_alreadyMerged(t *testing.T) {
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
prompt.StubConfirm(true)
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
cs.Stub("") // git symbolic-ref --quiet --short HEAD
cs.Stub("") // git checkout master
@ -587,10 +570,12 @@ func TestPRMerge_interactive(t *testing.T) {
Name: "deleteBranch",
Value: true,
},
{
Name: "isConfirmed",
Value: true,
},
})
prompt.StubConfirm(true)
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
t.Fatalf("Got unexpected error running `pr merge` %s", err)
@ -598,3 +583,51 @@ func TestPRMerge_interactive(t *testing.T) {
test.ExpectLines(t, output.Stderr(), "Merged pull request #3", `Deleted branch.*blueberries`)
}
func TestPRMerge_interactiveCancelled(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
http.Register(
httpmock.GraphQL(`query PullRequestForBranch\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "pullRequests": { "nodes": [{
"headRefName": "blueberries",
"headRepositoryOwner": {"login": "OWNER"},
"id": "THE-ID",
"number": 3
}] } } } }`))
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
cs.Stub("") // git symbolic-ref --quiet --short HEAD
cs.Stub("") // git checkout master
cs.Stub("") // git push origin --delete blueberries
cs.Stub("") // git branch -d
as, surveyTeardown := prompt.InitAskStubber()
defer surveyTeardown()
as.Stub([]*prompt.QuestionStub{
{
Name: "mergeMethod",
Value: 0,
},
{
Name: "deleteBranch",
Value: true,
},
{
Name: "isConfirmed",
Value: false,
},
})
output, err := runCommand(http, "blueberries", true, "")
if !errors.Is(err, cmdutil.SilentError) {
t.Fatalf("got error %v", err)
}
assert.Equal(t, "Cancelled.\n", output.Stderr())
}