From 326b678b2483ac723378800d75ba73fc77ce72b8 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 May 2020 08:51:22 -0700 Subject: [PATCH 01/30] Add delete branch Co-Authored-By: Nate Smith --- command/pr.go | 122 +++++++++++++++++++++++++++++++++++++++------ command/pr_test.go | 34 +++++++++++++ git/git.go | 12 +++++ 3 files changed, 153 insertions(+), 15 deletions(-) diff --git a/command/pr.go b/command/pr.go index e590c0e70..47b5f34e5 100644 --- a/command/pr.go +++ b/command/pr.go @@ -3,10 +3,12 @@ package command import ( "fmt" "io" + "os" "regexp" "strconv" "strings" + "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" @@ -471,36 +473,126 @@ func prMerge(cmd *cobra.Command, args []string) error { return err } - rebase, err := cmd.Flags().GetBool("rebase") - if err != nil { - return err - } - squash, err := cmd.Flags().GetBool("squash") - if err != nil { - return err + mergeMethod := api.PullRequestMergeMethodMerge + deleteBranch := true + + isInteractive := !cmd.Flags().Changed("rebase") && !cmd.Flags().Changed("squash") && !cmd.Flags().Changed("merged") + if isInteractive { + mergeMethod, deleteBranch, err = prInteractiveMerge() + if err != nil { + return nil + } + + } else { + rebase, err := cmd.Flags().GetBool("rebase") + if err != nil { + return err + } + squash, err := cmd.Flags().GetBool("squash") + if err != nil { + return err + } + + if rebase { + mergeMethod = api.PullRequestMergeMethodRebase + } else if squash { + mergeMethod = api.PullRequestMergeMethodSquash + } else { + mergeMethod = api.PullRequestMergeMethodMerge + } } - var output string - if rebase { - output = fmt.Sprintf("%s Rebased and merged pull request #%d\n", utils.Green("✔"), pr.Number) + var action string + if mergeMethod == api.PullRequestMergeMethodRebase { + action = "Rebased and merged" err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodRebase) - } else if squash { - output = fmt.Sprintf("%s Squashed and merged pull request #%d\n", utils.Green("✔"), pr.Number) + } else if mergeMethod == api.PullRequestMergeMethodSquash { + action = "Squashed and merged" err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodSquash) - } else { - output = fmt.Sprintf("%s Merged pull request #%d\n", utils.Green("✔"), pr.Number) + } else if mergeMethod == api.PullRequestMergeMethodMerge { + action = "Merged" err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodMerge) + } else { + fmt.Fprintf(os.Stderr, "Unknown merge method used\n") + os.Exit(1) } if err != nil { return fmt.Errorf("API call failed: %w", err) } - fmt.Fprint(colorableOut(cmd), output) + fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d\n", utils.Magenta("✔"), action, pr.Number) + + if deleteBranch { + branch, err := prDeleteCurrentBranch(baseRepo.(*api.Repository)) + if err != nil { + return err + } + fmt.Fprintf(colorableOut(cmd), "%s Deleted branch %s\n", utils.Red("✔"), utils.Cyan(branch)) + } return nil } +func prInteractiveMerge() (api.PullRequestMergeMethod, bool, error) { + mergeMethodQuestion := &survey.Question{ + Name: "mergeMethod", + Prompt: &survey.Select{ + Message: "What merge method would you like to use?", + Options: []string{"Create a merge commit", "Rebase and merge", "Squash and merge"}, + Default: "merge", + }, + } + + deleteBranchQuestion := &survey.Question{ + Name: "deleteBranch", + Prompt: &survey.Confirm{ + Message: "Delete the branch locally and on GitHub?", + Default: true, + }, + } + + qs := []*survey.Question{mergeMethodQuestion, deleteBranchQuestion} + + answers := struct { + MergeMethod int + DeleteBranch bool + }{} + + err := SurveyAsk(qs, &answers) + if err != nil { + return 0, false, fmt.Errorf("could not prompt: %w", err) + } + + var mergeMethod api.PullRequestMergeMethod + switch answers.MergeMethod { + case 0: + mergeMethod = api.PullRequestMergeMethodMerge + case 1: + mergeMethod = api.PullRequestMergeMethodRebase + case 2: + mergeMethod = api.PullRequestMergeMethodSquash + } + + deleteBranch := answers.DeleteBranch + return mergeMethod, deleteBranch, nil +} + +func prDeleteCurrentBranch(repo *api.Repository) (string, error) { + branch, err := git.CurrentBranch() + if err != nil { + return "", err + } + + err = git.CheckoutBranch(repo.DefaultBranchRef.Name) + if err != nil { + return "", err + } + + err = git.DeleteBranch(branch) + return branch, err +} + func printPrPreview(out io.Writer, pr *api.PullRequest) error { // Header (Title and State) fmt.Fprintln(out, utils.Bold(pr.Title)) diff --git a/command/pr_test.go b/command/pr_test.go index 61971d538..73f4c4015 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1061,3 +1061,37 @@ func TestPrMerge_alreadyMerged(t *testing.T) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) } } + +func TestPRMerge_interactive(t *testing.T) { + initWithStubs("feature", + stubResponse{200, bytes.NewBufferString(` + { "data": { + + } }`)}) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") + + as, surveyTeardown := initAskStubber() + defer surveyTeardown() + + as.Stub([]*QuestionStub{ + { + Name: "choose merge method", + Value: "merge", + }, + { + Name: "delete branch locally", + Default: true, + Value: "Y", + }, + }) + + output, err := RunCommand(`pr merge`) + eq(t, err, nil) + + stderr := string(output.Stderr()) + eq(t, strings.Contains(stderr, "warning: could not compute title or body defaults: could not find any commits"), true) +} diff --git a/git/git.go b/git/git.go index 360917eab..496508f97 100644 --- a/git/git.go +++ b/git/git.go @@ -203,6 +203,18 @@ func ReadBranchConfig(branch string) (cfg BranchConfig) { return } +func DeleteBranch(branch string) error { + configCmd := GitCommand("branch", "-d", branch) + _, err := run.PrepareCmd(configCmd).Output() + return err +} + +func CheckoutBranch(branch string) error { + configCmd := GitCommand("checkout", branch) + _, err := run.PrepareCmd(configCmd).Output() + return err +} + func isFilesystemPath(p string) bool { return p == "." || strings.HasPrefix(p, "./") || strings.HasPrefix(p, "/") } From fabbfe1da67484edeaec9bb9619bb2b26c7efc3b Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 May 2020 08:58:38 -0700 Subject: [PATCH 02/30] Add flag for deleting branch Co-Authored-By: Nate Smith --- command/pr.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index c147abab0..5fa277ad5 100644 --- a/command/pr.go +++ b/command/pr.go @@ -28,6 +28,7 @@ func init() { prCmd.AddCommand(prCloseCmd) prCmd.AddCommand(prReopenCmd) prCmd.AddCommand(prMergeCmd) + prMergeCmd.Flags().BoolP("delete", "d", true, "Delete the local branch after merge") prMergeCmd.Flags().BoolP("merge", "m", true, "Merge the commits with the base branch") prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch") prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch") @@ -473,7 +474,10 @@ func prMerge(cmd *cobra.Command, args []string) error { } mergeMethod := api.PullRequestMergeMethodMerge - deleteBranch := true + deleteBranch, err := cmd.Flags().GetBool("delete") + if err != nil { + return nil + } isInteractive := !cmd.Flags().Changed("rebase") && !cmd.Flags().Changed("squash") && !cmd.Flags().Changed("merged") if isInteractive { From 2328dffd9708733c81ac6de39de920890b90180b Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 May 2020 09:11:15 -0700 Subject: [PATCH 03/30] Add test Co-Authored-By: Nate Smith --- command/pr.go | 2 +- command/pr_test.go | 28 ++++++++++++++++------------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/command/pr.go b/command/pr.go index 5fa277ad5..071bea50c 100644 --- a/command/pr.go +++ b/command/pr.go @@ -531,7 +531,7 @@ func prMerge(cmd *cobra.Command, args []string) error { if err != nil { return err } - fmt.Fprintf(colorableOut(cmd), "%s Deleted branch %s\n", utils.Red("✔"), utils.Cyan(branch)) + fmt.Fprintf(colorableOut(cmd), "%s Deleted local branch %s\n", utils.Red("✔"), utils.Cyan(branch)) } return nil diff --git a/command/pr_test.go b/command/pr_test.go index 54883754c..3920cb4e6 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1095,15 +1095,19 @@ func TestPrMerge_alreadyMerged(t *testing.T) { } func TestPRMerge_interactive(t *testing.T) { - initWithStubs("feature", + initWithStubs("blueberries", stubResponse{200, bytes.NewBufferString(` - { "data": { - - } }`)}) + { "data": { "repository": { "pullRequests": { "nodes": [ + { "headRefName": "blueberries", "id": "THE-ID", "number": 3} + ] } } } }`)}, + stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)}) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() + cs.Stub("") + cs.Stub("") + cs.Stub("") cs.Stub("") as, surveyTeardown := initAskStubber() @@ -1111,19 +1115,19 @@ func TestPRMerge_interactive(t *testing.T) { as.Stub([]*QuestionStub{ { - Name: "choose merge method", - Value: "merge", + Name: "mergeMethod", + Value: 0, }, { - Name: "delete branch locally", - Default: true, - Value: "Y", + Name: "deleteBranch", + Value: true, }, }) output, err := RunCommand(`pr merge`) - eq(t, err, nil) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } - stderr := string(output.Stderr()) - eq(t, strings.Contains(stderr, "warning: could not compute title or body defaults: could not find any commits"), true) + test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted local branch") } From f0cd8301702f2f4c9d9d0209b98942bc26ea184b Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 May 2020 09:22:37 -0700 Subject: [PATCH 04/30] Fix merge flag Co-Authored-By: Nate Smith --- command/pr.go | 4 ++-- command/pr_test.go | 40 ++++++++++++++++++++++++++++++++++------ git/git.go | 2 +- test/helpers.go | 1 + 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/command/pr.go b/command/pr.go index 071bea50c..b0a47db3c 100644 --- a/command/pr.go +++ b/command/pr.go @@ -479,7 +479,7 @@ func prMerge(cmd *cobra.Command, args []string) error { return nil } - isInteractive := !cmd.Flags().Changed("rebase") && !cmd.Flags().Changed("squash") && !cmd.Flags().Changed("merged") + isInteractive := !cmd.Flags().Changed("rebase") && !cmd.Flags().Changed("squash") && !cmd.Flags().Changed("merge") if isInteractive { mergeMethod, deleteBranch, err = prInteractiveMerge() if err != nil { @@ -592,7 +592,7 @@ func prDeleteCurrentBranch(repo *api.Repository) (string, error) { return "", err } - err = git.DeleteBranch(branch) + err = git.DeleteLocalBranch(branch) return branch, err } diff --git a/command/pr_test.go b/command/pr_test.go index 3920cb4e6..9195364cd 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -996,7 +996,7 @@ func TestPrMerge(t *testing.T) { stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, ) - output, err := RunCommand("pr merge 1") + output, err := RunCommand("pr merge 1 --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -1013,6 +1013,10 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { defer cmdTeardown() 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 + cs.Stub("") // git checkout master + cs.Stub("") // git branch -d jsonFile, _ := os.Open("../test/fixtures/prViewPreviewWithMetadataByBranch.json") defer jsonFile.Close() @@ -1022,7 +1026,7 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, ) - output, err := RunCommand("pr merge") + output, err := RunCommand("pr merge --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -1042,6 +1046,14 @@ func TestPrMerge_rebase(t *testing.T) { stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, ) + 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 branch -d + output, err := RunCommand("pr merge 2 --rebase") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) @@ -1062,6 +1074,14 @@ func TestPrMerge_squash(t *testing.T) { stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, ) + 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 branch -d + output, err := RunCommand("pr merge 3 --squash") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) @@ -1082,6 +1102,14 @@ func TestPrMerge_alreadyMerged(t *testing.T) { stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, ) + 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 branch -d + output, err := RunCommand("pr merge 4") if err == nil { t.Fatalf("expected an error running command `pr merge`: %v", err) @@ -1105,10 +1133,10 @@ func TestPRMerge_interactive(t *testing.T) { cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() - cs.Stub("") - cs.Stub("") - cs.Stub("") - cs.Stub("") + 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 branch -d as, surveyTeardown := initAskStubber() defer surveyTeardown() diff --git a/git/git.go b/git/git.go index 496508f97..38a9a60bf 100644 --- a/git/git.go +++ b/git/git.go @@ -203,7 +203,7 @@ func ReadBranchConfig(branch string) (cfg BranchConfig) { return } -func DeleteBranch(branch string) error { +func DeleteLocalBranch(branch string) error { configCmd := GitCommand("branch", "-d", branch) _, err := run.PrepareCmd(configCmd).Output() return err diff --git a/test/helpers.go b/test/helpers.go index c8b0efe53..c2d4e2f37 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -68,6 +68,7 @@ func createStubbedPrepareCmd(cs *CmdStubber) func(*exec.Cmd) run.Runnable { if call >= len(cs.Stubs) { panic(fmt.Sprintf("more execs than stubs. most recent call: %v", cmd)) } + // fmt.Printf("Called stub for `%v`\n", cmd) // Helpful for debugging return cs.Stubs[call] } } From 214be16d428dcf7162a52ae4e79c187f85cf9a5e Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 May 2020 09:30:19 -0700 Subject: [PATCH 05/30] Add test for mutually exclusive flags Co-Authored-By: Nate Smith --- command/pr.go | 20 +++++++++++++++++++- command/pr_test.go | 23 +++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index b0a47db3c..b93976600 100644 --- a/command/pr.go +++ b/command/pr.go @@ -1,6 +1,7 @@ package command import ( + "errors" "fmt" "io" "os" @@ -479,7 +480,24 @@ func prMerge(cmd *cobra.Command, args []string) error { return nil } - isInteractive := !cmd.Flags().Changed("rebase") && !cmd.Flags().Changed("squash") && !cmd.Flags().Changed("merge") + // Ensure only one merge method is specified + found := 0 + isInteractive := false + if cmd.Flags().Changed("merge") { + found++ + } + if cmd.Flags().Changed("rebase") { + found++ + } + if cmd.Flags().Changed("squash") { + found++ + } + if found == 0 { + isInteractive = true + } else if found > 1 { + return errors.New("expected exactly one of --merge, --rebase, or --squash") + } + if isInteractive { mergeMethod, deleteBranch, err = prInteractiveMerge() if err != nil { diff --git a/command/pr_test.go b/command/pr_test.go index 9195364cd..f771c9ece 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -996,6 +996,15 @@ func TestPrMerge(t *testing.T) { stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, ) + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + 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 + cs.Stub("") // git checkout master + cs.Stub("") + output, err := RunCommand("pr merge 1 --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) @@ -1159,3 +1168,17 @@ func TestPRMerge_interactive(t *testing.T) { test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted local branch") } + +func TestPrMerge_multipleMergeMethods(t *testing.T) { + initWithStubs("master", + stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": { + "pullRequest": { "number": 1, "closed": false, "state": "OPEN"} + } } }`)}, + stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, + ) + + _, err := RunCommand("pr merge 1 --merge --squash") + if err == nil { + t.Fatal("expected error running `pr merge` with multiple merge methods") + } +} From 24dece9418a4205b9b2c8468b5686bceeb3da824 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 May 2020 09:45:22 -0700 Subject: [PATCH 06/30] Name flag `delete-branch` Co-Authored-By: Nate Smith --- command/pr.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/pr.go b/command/pr.go index b93976600..9f274333d 100644 --- a/command/pr.go +++ b/command/pr.go @@ -29,7 +29,7 @@ func init() { prCmd.AddCommand(prCloseCmd) prCmd.AddCommand(prReopenCmd) prCmd.AddCommand(prMergeCmd) - prMergeCmd.Flags().BoolP("delete", "d", true, "Delete the local branch after merge") + prMergeCmd.Flags().BoolP("delete-branch", "d", true, "Delete the local branch after merge") prMergeCmd.Flags().BoolP("merge", "m", true, "Merge the commits with the base branch") prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch") prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch") @@ -475,7 +475,7 @@ func prMerge(cmd *cobra.Command, args []string) error { } mergeMethod := api.PullRequestMergeMethodMerge - deleteBranch, err := cmd.Flags().GetBool("delete") + deleteBranch, err := cmd.Flags().GetBool("delete-branch") if err != nil { return nil } From 6e01081899a3acaccc9b39c45a7528cc03501070 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 May 2020 09:47:31 -0700 Subject: [PATCH 07/30] Add test for --delete-branch Co-Authored-By: Nate Smith --- command/pr_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/command/pr_test.go b/command/pr_test.go index f771c9ece..229dc9e24 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1017,6 +1017,30 @@ func TestPrMerge(t *testing.T) { } } +func TestPrMerge_deleteBranch(t *testing.T) { + initWithStubs("blueberries", + stubResponse{200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "headRefName": "blueberries", "id": "THE-ID", "number": 3} + ] } } } }`)}, + stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)}) + + 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 branch -d + + output, err := RunCommand(`pr merge --merge --delete-branch`) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted local branch") +} + func TestPrMerge_noPrNumberGiven(t *testing.T) { cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() From 6732ac01c9e9c2c969041727ced109c94aed3f82 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 May 2020 09:55:34 -0700 Subject: [PATCH 08/30] Fix var Co-Authored-By: Nate Smith --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 9f274333d..ea7d0bea5 100644 --- a/command/pr.go +++ b/command/pr.go @@ -474,7 +474,7 @@ func prMerge(cmd *cobra.Command, args []string) error { return err } - mergeMethod := api.PullRequestMergeMethodMerge + var mergeMethod api.PullRequestMergeMethod deleteBranch, err := cmd.Flags().GetBool("delete-branch") if err != nil { return nil From 4d539f937e0fb3c46303ce3a07e6b97e1b95974c Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 May 2020 08:05:05 -0700 Subject: [PATCH 09/30] Return err --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index ea7d0bea5..f1218ac1d 100644 --- a/command/pr.go +++ b/command/pr.go @@ -477,7 +477,7 @@ func prMerge(cmd *cobra.Command, args []string) error { var mergeMethod api.PullRequestMergeMethod deleteBranch, err := cmd.Flags().GetBool("delete-branch") if err != nil { - return nil + return err } // Ensure only one merge method is specified From a10755858369739b2354b7488bd7edf9c38cad19 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 May 2020 08:05:12 -0700 Subject: [PATCH 10/30] Default is now false --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index f1218ac1d..347f88f61 100644 --- a/command/pr.go +++ b/command/pr.go @@ -30,7 +30,7 @@ func init() { prCmd.AddCommand(prReopenCmd) prCmd.AddCommand(prMergeCmd) prMergeCmd.Flags().BoolP("delete-branch", "d", true, "Delete the local branch after merge") - prMergeCmd.Flags().BoolP("merge", "m", true, "Merge the commits with the base branch") + prMergeCmd.Flags().BoolP("merge", "m", false, "Merge the commits with the base branch") prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch") prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch") From d41778493b401c1c70ccb4c9c56244eb7c51ea15 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 May 2020 08:09:08 -0700 Subject: [PATCH 11/30] Only look for enabled flags --- command/pr.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/command/pr.go b/command/pr.go index 347f88f61..6de1e3640 100644 --- a/command/pr.go +++ b/command/pr.go @@ -481,21 +481,22 @@ func prMerge(cmd *cobra.Command, args []string) error { } // Ensure only one merge method is specified - found := 0 + flagCount := 0 isInteractive := false - if cmd.Flags().Changed("merge") { - found++ + if b, _ := cmd.Flags().GetBool("merge"); b { + flagCount++ } - if cmd.Flags().Changed("rebase") { - found++ + if b, _ := cmd.Flags().GetBool("rebase"); b { + flagCount++ } - if cmd.Flags().Changed("squash") { - found++ + if b, _ := cmd.Flags().GetBool("squash"); b { + flagCount++ } - if found == 0 { + + if flagCount == 0 { isInteractive = true - } else if found > 1 { - return errors.New("expected exactly one of --merge, --rebase, or --squash") + } else if flagCount > 1 { + return errors.New("expected exactly one of --merge, --rebase, or --squash to be true") } if isInteractive { From 587dd4d7977de452f53531ba9f063bbc0c083006 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 May 2020 08:09:27 -0700 Subject: [PATCH 12/30] a better name --- command/pr.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/command/pr.go b/command/pr.go index 6de1e3640..8b0ed32ec 100644 --- a/command/pr.go +++ b/command/pr.go @@ -481,21 +481,21 @@ func prMerge(cmd *cobra.Command, args []string) error { } // Ensure only one merge method is specified - flagCount := 0 + enabledFlagCount := 0 isInteractive := false if b, _ := cmd.Flags().GetBool("merge"); b { - flagCount++ + enabledFlagCount++ } if b, _ := cmd.Flags().GetBool("rebase"); b { - flagCount++ + enabledFlagCount++ } if b, _ := cmd.Flags().GetBool("squash"); b { - flagCount++ + enabledFlagCount++ } - if flagCount == 0 { + if enabledFlagCount == 0 { isInteractive = true - } else if flagCount > 1 { + } else if enabledFlagCount > 1 { return errors.New("expected exactly one of --merge, --rebase, or --squash to be true") } From feca9f6eaf0eef6eb70af58344e01581c12f6f05 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 May 2020 08:51:10 -0700 Subject: [PATCH 13/30] Add convertRepoInterfaceToRepository --- command/root.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/command/root.go b/command/root.go index ef88a3184..ca92c25a7 100644 --- a/command/root.go +++ b/command/root.go @@ -250,6 +250,30 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interfac return baseRepo, nil } +func convertRepoInterfaceToRepository(ctx context.Context, repo ghrepo.Interface) (*api.Repository, error) { + apiClient, err := apiClientForContext(ctx) + if err != nil { + return nil, err + } + + remotes, err := ctx.Remotes() + if err != nil { + return nil, err + } + + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") + if err != nil { + return nil, err + } + + baseRepo, err := repoContext.BaseRepo() + if err != nil { + return nil, err + } + + return baseRepo, nil +} + func rootHelpFunc(command *cobra.Command, s []string) { if command != RootCmd { cobraDefaultHelpFunc(command, s) From 848dd4444348896af1c64153df86f5aa3c9adf00 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 May 2020 08:51:37 -0700 Subject: [PATCH 14/30] Delete branch of PR, not the current branch --- command/pr.go | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/command/pr.go b/command/pr.go index 8b0ed32ec..e5c72b166 100644 --- a/command/pr.go +++ b/command/pr.go @@ -546,11 +546,21 @@ func prMerge(cmd *cobra.Command, args []string) error { fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d\n", utils.Magenta("✔"), action, pr.Number) if deleteBranch { - branch, err := prDeleteCurrentBranch(baseRepo.(*api.Repository)) + repo, err := convertRepoInterfaceToRepository(ctx, baseRepo) if err != nil { return err } - fmt.Fprintf(colorableOut(cmd), "%s Deleted local branch %s\n", utils.Red("✔"), utils.Cyan(branch)) + + err = git.CheckoutBranch(repo.DefaultBranchRef.Name) + if err != nil { + return err + } + + err = git.DeleteLocalBranch(pr.HeadRefName) + if err != nil { + return err + } + fmt.Fprintf(colorableOut(cmd), "%s Deleted local branch %s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName)) } return nil @@ -600,21 +610,6 @@ func prInteractiveMerge() (api.PullRequestMergeMethod, bool, error) { return mergeMethod, deleteBranch, nil } -func prDeleteCurrentBranch(repo *api.Repository) (string, error) { - branch, err := git.CurrentBranch() - if err != nil { - return "", err - } - - err = git.CheckoutBranch(repo.DefaultBranchRef.Name) - if err != nil { - return "", err - } - - err = git.DeleteLocalBranch(branch) - return branch, err -} - func printPrPreview(out io.Writer, pr *api.PullRequest) error { // Header (Title and State) fmt.Fprintln(out, utils.Bold(pr.Title)) From c645fd5ae58d1dda37a581c8e46999d4609fd6dd Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 May 2020 10:33:54 -0700 Subject: [PATCH 15/30] Better errors around branch deletion --- command/pr.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index e5c72b166..21b850a67 100644 --- a/command/pr.go +++ b/command/pr.go @@ -558,7 +558,8 @@ func prMerge(cmd *cobra.Command, args []string) error { err = git.DeleteLocalBranch(pr.HeadRefName) if err != nil { - return err + fmt.Fprintf(colorableErr(cmd), "%s Could not deleted local branch %s: %s\n", utils.Red("!"), utils.Cyan(pr.HeadRefName), err) + return nil } fmt.Fprintf(colorableOut(cmd), "%s Deleted local branch %s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName)) } From 90664792b69f2df319594bf680e0e577513f36d4 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 May 2020 10:42:58 -0700 Subject: [PATCH 16/30] Fix message --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 21b850a67..148cbb074 100644 --- a/command/pr.go +++ b/command/pr.go @@ -580,7 +580,7 @@ func prInteractiveMerge() (api.PullRequestMergeMethod, bool, error) { deleteBranchQuestion := &survey.Question{ Name: "deleteBranch", Prompt: &survey.Confirm{ - Message: "Delete the branch locally and on GitHub?", + Message: "Delete the branch locally?", Default: true, }, } From ea164a8d489ecd24f7746db01de1f752a09d3184 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 May 2020 10:52:26 -0700 Subject: [PATCH 17/30] Fix test --- command/pr_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr_test.go b/command/pr_test.go index 229dc9e24..2310be2a5 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -982,10 +982,10 @@ func initWithStubs(branch string, stubs ...stubResponse) { initBlankContext("", "OWNER/REPO", branch) http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - for _, s := range stubs { http.StubResponse(s.ResponseCode, s.ResponseBody) } + http.StubRepoResponse("OWNER", "REPO") } func TestPrMerge(t *testing.T) { From 6666adbdceccc5bd45e886734b7e3a074e999b17 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 19 May 2020 11:29:00 -0700 Subject: [PATCH 18/30] Deal with merge conflicts --- api/queries_pr.go | 4 ++++ command/pr.go | 18 ++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index c1afc6bc9..abfb9485e 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -45,6 +45,7 @@ type PullRequest struct { BaseRefName string HeadRefName string Body string + Mergeable string Author struct { Login string @@ -228,6 +229,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu state url headRefName + mergeable headRepositoryOwner { login } @@ -382,6 +384,7 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu state closed body + mergeable author { login } @@ -490,6 +493,7 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea title state body + mergeable author { login } diff --git a/command/pr.go b/command/pr.go index 148cbb074..fadcda594 100644 --- a/command/pr.go +++ b/command/pr.go @@ -449,7 +449,15 @@ func prMerge(cmd *cobra.Command, args []string) error { var pr *api.PullRequest if len(args) > 0 { - pr, err = prFromArg(apiClient, baseRepo, args[0]) + var prNumber string + n, _ := prFromURL(args[0]) + if n != "" { + prNumber = n + } else { + prNumber = args[0] + } + + pr, err = prFromArg(apiClient, baseRepo, prNumber) if err != nil { return err } @@ -469,7 +477,13 @@ func prMerge(cmd *cobra.Command, args []string) error { } } - if pr.State == "MERGED" { + if pr.Mergeable == "CONFLICTING" { + err := fmt.Errorf("%s Pull request #%d has conflicts and isn't mergeable ", utils.Red("!"), pr.Number) + return err + } else if pr.Mergeable == "UNKNOWN" { + err := fmt.Errorf("%s Pull request #%d can't be merged right now", utils.Red("!"), pr.Number) + return err + } else if pr.State == "MERGED" { err := fmt.Errorf("%s Pull request #%d was already merged", utils.Red("!"), pr.Number) return err } From 96e3d79f5e7d0e0846ce6a20cb5bbc3913277415 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 19 May 2020 13:39:31 -0700 Subject: [PATCH 19/30] add "try again" text --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index fadcda594..408370a8b 100644 --- a/command/pr.go +++ b/command/pr.go @@ -481,7 +481,7 @@ func prMerge(cmd *cobra.Command, args []string) error { err := fmt.Errorf("%s Pull request #%d has conflicts and isn't mergeable ", utils.Red("!"), pr.Number) return err } else if pr.Mergeable == "UNKNOWN" { - err := fmt.Errorf("%s Pull request #%d can't be merged right now", utils.Red("!"), pr.Number) + err := fmt.Errorf("%s Pull request #%d can't be merged right now; try again in a few seconds", utils.Red("!"), pr.Number) return err } else if pr.State == "MERGED" { err := fmt.Errorf("%s Pull request #%d was already merged", utils.Red("!"), pr.Number) From 5c113d02c8e75277026a602dfbe24580335cd106 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 19 May 2020 13:39:36 -0700 Subject: [PATCH 20/30] Big M --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 408370a8b..9e4d33d6a 100644 --- a/command/pr.go +++ b/command/pr.go @@ -587,7 +587,7 @@ func prInteractiveMerge() (api.PullRequestMergeMethod, bool, error) { Prompt: &survey.Select{ Message: "What merge method would you like to use?", Options: []string{"Create a merge commit", "Rebase and merge", "Squash and merge"}, - Default: "merge", + Default: "Merge", }, } From 9185bf9a771e0a4f0f7409fc3ee1f988ae0d1f9d Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 19 May 2020 13:52:22 -0700 Subject: [PATCH 21/30] Return an error --- command/pr.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/command/pr.go b/command/pr.go index 9e4d33d6a..4ae0cf8c7 100644 --- a/command/pr.go +++ b/command/pr.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "io" - "os" "regexp" "sort" "strconv" @@ -549,8 +548,8 @@ func prMerge(cmd *cobra.Command, args []string) error { action = "Merged" err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodMerge) } else { - fmt.Fprintf(os.Stderr, "Unknown merge method used\n") - os.Exit(1) + err = fmt.Errorf("unknown merge method (%d) used", mergeMethod) + return err } if err != nil { From 1eedfe18bbb81af2fb5ced5b342d696515e87efc Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 20 May 2020 08:26:34 -0700 Subject: [PATCH 22/30] Update default text --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 4ae0cf8c7..d5527057b 100644 --- a/command/pr.go +++ b/command/pr.go @@ -586,7 +586,7 @@ func prInteractiveMerge() (api.PullRequestMergeMethod, bool, error) { Prompt: &survey.Select{ Message: "What merge method would you like to use?", Options: []string{"Create a merge commit", "Rebase and merge", "Squash and merge"}, - Default: "Merge", + Default: "Create a merge commit", }, } From 76368f92ee8e8615ac296c10448a17f5b86acba4 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 20 May 2020 08:32:08 -0700 Subject: [PATCH 23/30] Return error --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 869174303..2747dc5e6 100644 --- a/command/pr.go +++ b/command/pr.go @@ -578,7 +578,7 @@ func prMerge(cmd *cobra.Command, args []string) error { err = git.DeleteLocalBranch(pr.HeadRefName) if err != nil { fmt.Fprintf(colorableErr(cmd), "%s Could not deleted local branch %s: %s\n", utils.Red("!"), utils.Cyan(pr.HeadRefName), err) - return nil + return err } fmt.Fprintf(colorableOut(cmd), "%s Deleted local branch %s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName)) } From b4783dd192555fe8339a111a993026b8d9a0cae6 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 20 May 2020 09:00:39 -0700 Subject: [PATCH 24/30] don't always switch to the default branch --- command/pr.go | 9 ++++++++- command/pr_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 2747dc5e6..f99abbde0 100644 --- a/command/pr.go +++ b/command/pr.go @@ -570,11 +570,18 @@ func prMerge(cmd *cobra.Command, args []string) error { return err } - err = git.CheckoutBranch(repo.DefaultBranchRef.Name) + currentBranch, err := ctx.Branch() if err != nil { return err } + if currentBranch == pr.HeadRefName { + err = git.CheckoutBranch(repo.DefaultBranchRef.Name) + if err != nil { + return err + } + } + err = git.DeleteLocalBranch(pr.HeadRefName) if err != nil { fmt.Fprintf(colorableErr(cmd), "%s Could not deleted local branch %s: %s\n", utils.Red("!"), utils.Cyan(pr.HeadRefName), err) diff --git a/command/pr_test.go b/command/pr_test.go index 518e23235..d9b535c5e 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1041,6 +1041,30 @@ func TestPrMerge_deleteBranch(t *testing.T) { test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted local branch") } +func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { + initBlankContext("", "OWNER/REPO", "another-branch") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "headRefName": "blueberries", "id": "THE-ID", "number": 3} + ] } } } }`)) + http.StubResponse(200, bytes.NewBufferString(`{ "data": {} }`)) + http.StubRepoResponse("OWNER", "REPO") + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + // We don't expect the default branch to be checked out, just that blueberries is deleted + cs.Stub("") // git branch -d blueberries + + output, err := RunCommand(`pr merge --merge --delete-branch blueberries`) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted local branch") +} + func TestPrMerge_noPrNumberGiven(t *testing.T) { cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() From 5d99c5645633b17272ae17ae6d98f325b2a333a2 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 20 May 2020 09:21:59 -0700 Subject: [PATCH 25/30] combine conditionals --- command/pr.go | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/command/pr.go b/command/pr.go index f99abbde0..3f78d2340 100644 --- a/command/pr.go +++ b/command/pr.go @@ -504,12 +504,15 @@ func prMerge(cmd *cobra.Command, args []string) error { isInteractive := false if b, _ := cmd.Flags().GetBool("merge"); b { enabledFlagCount++ + mergeMethod = api.PullRequestMergeMethodMerge } if b, _ := cmd.Flags().GetBool("rebase"); b { enabledFlagCount++ + mergeMethod = api.PullRequestMergeMethodRebase } if b, _ := cmd.Flags().GetBool("squash"); b { enabledFlagCount++ + mergeMethod = api.PullRequestMergeMethodSquash } if enabledFlagCount == 0 { @@ -524,23 +527,6 @@ func prMerge(cmd *cobra.Command, args []string) error { return nil } - } else { - rebase, err := cmd.Flags().GetBool("rebase") - if err != nil { - return err - } - squash, err := cmd.Flags().GetBool("squash") - if err != nil { - return err - } - - if rebase { - mergeMethod = api.PullRequestMergeMethodRebase - } else if squash { - mergeMethod = api.PullRequestMergeMethodSquash - } else { - mergeMethod = api.PullRequestMergeMethodMerge - } } var action string From 46a1e3cd53e55108ea2faf24ec1b12524add5849 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 20 May 2020 11:19:31 -0700 Subject: [PATCH 26/30] Remove convertRepoInterfaceToRepository --- api/queries_repo.go | 3 +++ command/pr.go | 3 +-- command/root.go | 24 ------------------------ 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index c8ba31348..22d42cfbb 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -84,6 +84,9 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { hasIssuesEnabled description viewerPermission + defaultBranchRef { + name + } } }` variables := map[string]interface{}{ diff --git a/command/pr.go b/command/pr.go index 3f78d2340..464894c22 100644 --- a/command/pr.go +++ b/command/pr.go @@ -526,7 +526,6 @@ func prMerge(cmd *cobra.Command, args []string) error { if err != nil { return nil } - } var action string @@ -551,7 +550,7 @@ func prMerge(cmd *cobra.Command, args []string) error { fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d\n", utils.Magenta("✔"), action, pr.Number) if deleteBranch { - repo, err := convertRepoInterfaceToRepository(ctx, baseRepo) + repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { return err } diff --git a/command/root.go b/command/root.go index 48c9b50ed..6233be957 100644 --- a/command/root.go +++ b/command/root.go @@ -249,30 +249,6 @@ func determineBaseRepo(apiClient *api.Client, cmd *cobra.Command, ctx context.Co return baseRepo, nil } -func convertRepoInterfaceToRepository(ctx context.Context, repo ghrepo.Interface) (*api.Repository, error) { - apiClient, err := apiClientForContext(ctx) - if err != nil { - return nil, err - } - - remotes, err := ctx.Remotes() - if err != nil { - return nil, err - } - - repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") - if err != nil { - return nil, err - } - - baseRepo, err := repoContext.BaseRepo() - if err != nil { - return nil, err - } - - return baseRepo, nil -} - func rootHelpFunc(command *cobra.Command, s []string) { if command != RootCmd { cobraDefaultHelpFunc(command, s) From 41e67aa3e71bb95056afd4224a72a5302e4b324e Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 22 May 2020 10:04:02 -0700 Subject: [PATCH 27/30] Fix typo --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 464894c22..92cd8972d 100644 --- a/command/pr.go +++ b/command/pr.go @@ -569,7 +569,7 @@ func prMerge(cmd *cobra.Command, args []string) error { err = git.DeleteLocalBranch(pr.HeadRefName) if err != nil { - fmt.Fprintf(colorableErr(cmd), "%s Could not deleted local branch %s: %s\n", utils.Red("!"), utils.Cyan(pr.HeadRefName), err) + fmt.Fprintf(colorableErr(cmd), "%s Could not delete local branch %s: %s\n", utils.Red("!"), utils.Cyan(pr.HeadRefName), err) return err } fmt.Fprintf(colorableOut(cmd), "%s Deleted local branch %s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName)) From b2c1b12bee432d552c1442d46bb861a83369ee5b Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 22 May 2020 14:39:53 -0700 Subject: [PATCH 28/30] Don't delete branch if the repo flag is used --- command/pr.go | 2 +- command/pr_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 92cd8972d..c740b3968 100644 --- a/command/pr.go +++ b/command/pr.go @@ -549,7 +549,7 @@ func prMerge(cmd *cobra.Command, args []string) error { fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d\n", utils.Magenta("✔"), action, pr.Number) - if deleteBranch { + if deleteBranch && !cmd.Flags().Changed("repo") { repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { return err diff --git a/command/pr_test.go b/command/pr_test.go index d9b535c5e..5f5cdf6b8 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1017,6 +1017,31 @@ func TestPrMerge(t *testing.T) { } } +func TestPrMerge_withRepoFlag(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(`{ "data": { "repository": { + "pullRequest": { "number": 1, "closed": false, "state": "OPEN"} + } } }`)) + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + eq(t, len(cs.Calls), 0) + + output, err := RunCommand("pr merge 1 --merge -R stinky/boi") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + r := regexp.MustCompile(`Merged pull request #1`) + + if !r.MatchString(output.String()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + func TestPrMerge_deleteBranch(t *testing.T) { initWithStubs("blueberries", stubResponse{200, bytes.NewBufferString(` From 28d8a9e7819eaf33c9ea0ae2ed35c5a636b38d3f Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 22 May 2020 14:40:38 -0700 Subject: [PATCH 29/30] Use big D --- git/git.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/git.go b/git/git.go index 38a9a60bf..256b65f6d 100644 --- a/git/git.go +++ b/git/git.go @@ -204,7 +204,7 @@ func ReadBranchConfig(branch string) (cfg BranchConfig) { } func DeleteLocalBranch(branch string) error { - configCmd := GitCommand("branch", "-d", branch) + configCmd := GitCommand("branch", "-D", branch) _, err := run.PrepareCmd(configCmd).Output() return err } From d7c933bc40715fe2bb6813b9c78b82d858721ee9 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 22 May 2020 14:52:29 -0700 Subject: [PATCH 30/30] branchCmd --- git/git.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/git/git.go b/git/git.go index 256b65f6d..634b9e084 100644 --- a/git/git.go +++ b/git/git.go @@ -204,14 +204,14 @@ func ReadBranchConfig(branch string) (cfg BranchConfig) { } func DeleteLocalBranch(branch string) error { - configCmd := GitCommand("branch", "-D", branch) - _, err := run.PrepareCmd(configCmd).Output() + branchCmd := GitCommand("branch", "-D", branch) + err := run.PrepareCmd(branchCmd).Run() return err } func CheckoutBranch(branch string) error { configCmd := GitCommand("checkout", branch) - _, err := run.PrepareCmd(configCmd).Output() + err := run.PrepareCmd(configCmd).Run() return err }