8000 Remote delete by probablycorey · Pull Request #982 · cli/cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remote delete #982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
May 26, 2020
Merged

Remote delete #982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a303dab
Add remote deletion
probablycorey May 20, 2020
904c541
Only delete local branch if it exists
probablycorey May 20, 2020
80bdd1e
print error
probablycorey May 20, 2020
ae70a81
Update tests
probablycorey May 20, 2020
479707d
Merge remote-tracking branch 'origin/master' into remote-delete
probablycorey May 21, 2020
a8fa9a2
Merge remote-tracking branch 'origin/merge-interactive-merge' into re…
probablycorey May 21, 2020
368aacb
Fully qualify that ref
probablycorey May 22, 2020
858cd70
Update name
probablycorey May 22, 2020
e643765
Merge remote-tracking branch 'origin/merge-interactive-merge' into re…
probablycorey May 22, 2020
ff97fc1
Still delete remote branch if repo flag is used
probablycorey May 22, 2020
9113092
Delete via api
probablycorey May 22, 2020
9c88cd6
Whoops
probablycorey May 22, 2020
9c48e2d
Delete the branch
probablycorey May 22, 2020
9a290d7
Change interactive merge language
probablycorey May 22, 2020
9c075ab
Fix tests
probablycorey May 23, 2020
b27df34
Don't delete remote or local branches for forks probablycorey May 26, 2020
909e177
remove mergeable from queries that don't need it
probablycorey May 26, 2020
6c5c7cf
fix test
probablycorey May 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion api/queries_pr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
state
url
headRefName
mergeable
headRepositoryOwner {
login
}
Expand Down Expand Up @@ -1014,6 +1013,14 @@ func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) er
return err
}

func BranchDeleteRemote(client *Client, repo ghrepo.Interface, branch string) error {
var response struct {
NodeID string `json:"node_id"`
}
path := fmt.Sprintf("repos/%s/%s/git/refs/heads/%s", repo.RepoOwner(), repo.RepoName(), branch)
return client.REST("DELETE", path, nil, &response)
}

func min(a, b int) int {
if a < b {
return a
Expand Down
75 changes: 55 additions & 20 deletions command/pr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func init() {
prCmd.AddCommand(prCloseCmd)
prCmd.AddCommand(prReopenCmd)
prCmd.AddCommand(prMergeCmd)
prMergeCmd.Flags().BoolP("delete-branch", "d", true, "Delete the local branch after merge")
prMergeCmd.Flags().BoolP("delete-branch", "d", true, "Delete the local and remote branch after merge")
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")
Expand Down Expand Up @@ -499,6 +499,9 @@ func prMerge(cmd *cobra.Command, args []string) error {
return err
}

deleteLocalBranch := !cmd.Flags().Changed("repo")
crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner()

// Ensure only one merge method is specified
enabledFlagCount := 0
isInteractive := false
Expand All @@ -522,7 +525,7 @@ func prMerge(cmd *cobra.Command, args []string) error {
}

if isInteractive {
mergeMethod, deleteBranch, err = prInteractiveMerge()
mergeMethod, deleteBranch, err = prInteractiveMerge(deleteLocalBranch, crossRepoPR)
if err != nil {
return nil
}
Expand All @@ -549,7 +552,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 && !cmd.Flags().Changed("repo") {
if deleteBranch {
repo, err := api.GitHubRepo(apiClient, baseRepo)
if err != nil {
return err
Expand All @@ -560,25 +563,47 @@ func prMerge(cmd *cobra.Command, args []string) error {
return err
}

if currentBranch == pr.HeadRefName {
err = git.CheckoutBranch(repo.DefaultBranchRef.Name)
branchSwitchString := ""

if deleteLocalBranch && !crossRepoPR {
var branchToSwitchTo string
if currentBranch == pr.HeadRefName {
branchToSwitchTo = repo.DefaultBranchRef.Name
err = git.CheckoutBranch(repo.DefaultBranchRef.Name)
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)
if err != nil {
err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err)
return err
}
}

err = git.DeleteLocalBranch(pr.HeadRefName)
if err != nil {
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))
fmt.Fprintf(colorableOut(cmd), "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString)
}

return nil
}

func prInteractiveMerge() (api.PullRequestMergeMethod, bool, error) {
func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullRequestMergeMethod, bool, error) {
mergeMethodQuestion := &survey.Question{
Name: "mergeMethod",
Prompt: &survey.Select{
Expand All @@ -588,15 +613,25 @@ func prInteractiveMerge() (api.PullRequestMergeMethod, bool, error) {
},
}

deleteBranchQuestion := &survey.Question{
Name: "deleteBranch",
Prompt: &survey.Confirm{
Message: "Delete the branch locally?",
Default: true,
},
}
qs := []*survey.Question{mergeMethodQuestion}

if !crossRepoPR {
var message string
if deleteLocalBranch {
message = "Delete the branch locally and on GitHub?"
} else {
message = "Delete the branch on GitHub?"
}

qs := []*survey.Question{mergeMethodQuestion, deleteBranchQuestion}
deleteBranchQuestion := &survey.Question{
Name: "deleteBranch",
Prompt: &survey.Confirm{
Message: message,
Default: true,
},
}
qs = append(qs, deleteBranchQuestion)
}

answers := struct {
MergeMethod int
Expand Down
31 changes: 23 additions & 8 deletions command/pr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,7 @@ func TestPrMerge(t *testing.T) {
"pullRequest": { "number": 1, "closed": false, "state": "OPEN"}
} } }`)},
stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)},
)

cs, cmdTeardown := test.InitCmdStubber()
Expand Down Expand Up @@ -1021,9 +1022,11 @@ 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"}
"pullRequest": { "number": 1, "closed": false, "state": "OPEN"}
} } }`))
http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`))
http.StubResponse(200, bytes.NewBufferString(`{ "data": {} }`))
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`{"node_id": "THE-ID"}`))

cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
Expand All @@ -1048,22 +1051,25 @@ func TestPrMerge_deleteBranch(t *testing.T) {
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "headRefName": "blueberries", "id": "THE-ID", "number": 3}
] } } } }`)},
stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)})
stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)},
stubResponse{200, bytes.NewBufferString(`{"node_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 rev-parse --verify blueberries`
cs.Stub("") // git branch -d
cs.Stub("") // git push origin --delete blueberries

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")
test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted branch blueberries")
}

func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
Expand All @@ -1075,19 +1081,22 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
{ "headRefName": "blueberries", "id": "THE-ID", "number": 3}
] } } } }`))
http.StubResponse(200, bytes.NewBufferString(`{ "data": {} }`))
http.StubResponse(200, bytes.NewBufferString(`{"node_id": "THE-ID"}`))
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 rev-parse --verify blueberries
cs.Stub("") // git branch -d blueberries
cs.Stub("") // git push origin --delete 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")
test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted branch blueberries")
}

func TestPrMerge_noPrNumberGiven(t *testing.T) {
Expand All @@ -1106,6 +1115,7 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) {
initWithStubs("blueberries",
stubResponse{200, jsonFile},
stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)},
)

output, err := RunCommand("pr merge --merge")
Expand All @@ -1126,6 +1136,7 @@ func TestPrMerge_rebase(t *testing.T) {
"pullRequest": { "number": 2, "closed": false, "state": "OPEN"}
} } }`)},
stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)},
)

cs, cmdTeardown := test.InitCmdStubber()
Expand Down Expand Up @@ -1154,6 +1165,7 @@ func TestPrMerge_squash(t *testing.T) {
"pullRequest": { "number": 3, "closed": false, "state": "OPEN"}
} } }`)},
stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)},
)

cs, cmdTeardown := test.InitCmdStubber()
Expand Down Expand Up @@ -1182,6 +1194,7 @@ func TestPrMerge_alreadyMerged(t *testing.T) {
"pullRequest": { "number": 4, "closed": true, "state": "MERGED"}
} } }`)},
stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)},
)

cs, cmdTeardown := test.InitCmdStubber()
Expand All @@ -1208,8 +1221,9 @@ func TestPRMerge_interactive(t *testing.T) {
initWithStubs("blueberries",
stubResponse{200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "headRefName": "blueberries", "id": "THE-ID", "number": 3}
{ "headRefName": "blueberries", "headRepositoryOwner": {"login": "OWNER"}, "id": "THE-ID", "number": 3}
] } } } }`)},
stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)},
stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)})

cs, cmdTeardown := test.InitCmdStubber()
Expand All @@ -1218,6 +1232,7 @@ func TestPRMerge_interactive(t *testing.T) {
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 := initAskStubber()
Expand All @@ -1239,7 +1254,7 @@ func TestPRMerge_interactive(t *testing.T) {
t.Fatalf("Got unexpected error running `pr merge` %s", err)
}

test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted local branch")
test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted branch blueberries")
}

func TestPrMerge_multipleMergeMethods(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions git/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,12 @@ func DeleteLocalBranch(branch string) error {
return err
}

func HasLocalBranch(branch string) bool {
configCmd := GitCommand("rev-parse", "--verify", "refs/heads/"+branch)
_, err := run.PrepareCmd(configCmd).Output()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip: to run a command without being interested in its output, you can use Run() instead of Output().

return err == nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the assumption here that any error means the branch doesn't exist. I figured that will be more stable in the future than doing a string match on the error message.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a fair assumption! rev-parse --verify is sufficiently low-level

}

func CheckoutBranch(branch string) error {
configCmd := GitCommand("checkout", branch)
err := run.PrepareCmd(configCmd).Run()
Expand Down
0