Delete remote branch after closing pull request.

Based on https://github.com/levibostian/spr/tree/delete-closed-branches
but with a feature flag, separate unit tests and documentation update.

Author: efarrer

config/config.go

@@ -51,6 +51,7 @@ type UserConfig struct {
 	CreateDraftPRs       bool `default:"false" yaml:"createDraftPRs"`
 	PreserveTitleAndBody bool `default:"false" yaml:"preserveTitleAndBody"`
 	NoRebase             bool `default:"false" yaml:"noRebase"`
+	DeleteMergedBranches bool `default:"false" yaml:"deleteMergedBranches"`
 }
 
 type InternalState struct {

git/helpers.go

@@ -42,6 +42,12 @@ func GetLocalTopCommit(cfg *config.Config, gitcmd GitInterface) *Commit {
 	return &commits[len(commits)-1]
 }
 
+func DeleteRemoteBranch(cfg *config.Config, gitcmd GitInterface, branchName string) {
+	command := fmt.Sprintf("push origin --delete %s", branchName)
+	err := gitcmd.Git(command, nil)
+	check(err)
+}
+
 // GetLocalCommitStack returns a list of unmerged commits
 //
 //	the list is ordered with the bottom commit in the stack first

git/mockgit/mockgit.go

@@ -74,6 +74,10 @@ func (m *Mock) ExpectFetch() {
 	m.expect("git rebase origin/master --autostash")
 }
 
+func (m *Mock) ExpectDeleteBranch(branchName string) {
+	m.expect(fmt.Sprintf("git push origin --delete %s", branchName))
+}
+
 func (m *Mock) ExpectLogAndRespond(commits []*git.Commit) {
 	m.expect("git log --format=medium --no-color origin/master..HEAD").commitRespond(commits)
 }

readme.md

@@ -187,6 +187,7 @@ User specific configuration is saved to .spr.yml in the user home directory.
 | createDraftPRs       | bool | false   | new pull requests are created as draft |
 | preserveTitleAndBody | bool | false   | updating pull requests will not overwrite the pr title and body |
 | noRebase             | bool | false   | when true spr update will not rebase on top of origin |
+| deleteMergedBranches | bool | false   | delete branches after prs are merged |
 
 Happy Coding!
 -------------

spr/spr.go

@@ -317,7 +317,9 @@ func (sd *stackediff) MergePullRequests(ctx context.Context, count *uint) {
 	mergeMethod, err := sd.config.MergeMethod()
 	check(err)
 	sd.github.MergePullRequest(ctx, prToMerge, mergeMethod)
-	sd.profiletimer.Step("MergePullRequests::merge pr")
+	if sd.config.User.DeleteMergedBranches {
+		git.DeleteRemoteBranch(sd.config, sd.gitcmd, prToMerge.FromBranch)
+	}
 
 	// Close all the pull requests in the stack below the merged pr
 	//  Before closing add a review comment with the pr that merged the commit.
@@ -328,6 +330,9 @@ func (sd *stackediff) MergePullRequests(ctx context.Context, count *uint) {
 			prToMerge.Number, sd.config.Repo.GitHubHost, sd.config.Repo.GitHubRepoOwner, sd.config.Repo.GitHubRepoName, prToMerge.Number)
 		sd.github.CommentPullRequest(ctx, pr, comment)
 		sd.github.ClosePullRequest(ctx, pr)
+		if sd.config.User.DeleteMergedBranches {
+			git.DeleteRemoteBranch(sd.config, sd.gitcmd, pr.FromBranch)
+		}
 	}
 	sd.profiletimer.Step("MergePullRequests::close prs")
 

spr/spr_test.go

@@ -343,6 +343,72 @@ func TestSPRBasicFlowFourCommits(t *testing.T) {
 	output.Reset()
 }
 
+func TestSPRBasicFlowDeleteBranch(t *testing.T) {
+	s, gitmock, githubmock, _, output := makeTestObjects(t)
+	s.config.User.DeleteMergedBranches = true
+	assert := require.New(t)
+	ctx := context.Background()
+
+	c1 := git.Commit{
+		CommitID:   "00000001",
+		CommitHash: "c100000000000000000000000000000000000000",
+		Subject:    "test commit 1",
+	}
+	c2 := git.Commit{
+		CommitID:   "00000002",
+		CommitHash: "c200000000000000000000000000000000000000",
+		Subject:    "test commit 2",
+	}
+
+	// 'git spr update' :: UpdatePullRequest :: commits=[c1]
+	githubmock.ExpectGetInfo()
+	gitmock.ExpectFetch()
+	gitmock.ExpectLogAndRespond([]*git.Commit{&c1})
+	gitmock.ExpectPushCommits([]*git.Commit{&c1})
+	githubmock.ExpectCreatePullRequest(c1, nil)
+	githubmock.ExpectGetAssignableUsers()
+	githubmock.ExpectAddReviewers([]string{mockclient.NobodyUserID})
+	githubmock.ExpectUpdatePullRequest(c1, nil)
+	githubmock.ExpectGetInfo()
+	s.UpdatePullRequests(ctx, []string{mockclient.NobodyLogin}, nil)
+	fmt.Printf("OUT: %s\n", output.String())
+	assert.Equal("[vvvv]   1 : test commit 1\n", output.String())
+	output.Reset()
+
+	// 'git spr update' :: UpdatePullRequest :: commits=[c1, c2]
+	githubmock.ExpectGetInfo()
+	gitmock.ExpectFetch()
+	gitmock.ExpectLogAndRespond([]*git.Commit{&c2, &c1})
+	gitmock.ExpectPushCommits([]*git.Commit{&c2})
+	githubmock.ExpectCreatePullRequest(c2, &c1)
+	githubmock.ExpectGetAssignableUsers()
+	githubmock.ExpectAddReviewers([]string{mockclient.NobodyUserID})
+	githubmock.ExpectUpdatePullRequest(c1, nil)
+	githubmock.ExpectUpdatePullRequest(c2, &c1)
+	githubmock.ExpectGetInfo()
+	s.UpdatePullRequests(ctx, []string{mockclient.NobodyLogin}, nil)
+	lines := strings.Split(output.String(), "\n")
+	fmt.Printf("OUT: %s\n", output.String())
+	assert.Equal("warning: not updating reviewers for PR #1", lines[0])
+	assert.Equal("[vvvv]   1 : test commit 2", lines[1])
+	assert.Equal("[vvvv]   1 : test commit 1", lines[2])
+	output.Reset()
+
+	// 'git spr merge' :: MergePullRequest :: commits=[a1, a2]
+	githubmock.ExpectGetInfo()
+	githubmock.ExpectUpdatePullRequest(c2, nil)
+	githubmock.ExpectMergePullRequest(c2, genclient.PullRequestMergeMethod_REBASE)
+	gitmock.ExpectDeleteBranch("from_branch") // <--- This is the key expectation of this test.
+	githubmock.ExpectCommentPullRequest(c1)
+	githubmock.ExpectClosePullRequest(c1)
+	gitmock.ExpectDeleteBranch("from_branch") // <--- This is the key expectation of this test.
+	s.MergePullRequests(ctx, nil)
+	lines = strings.Split(output.String(), "\n")
+	assert.Equal("MERGED   1 : test commit 1", lines[0])
+	fmt.Printf("OUT: %s\n", output.String())
+	output.Reset()
+}
+
 func TestSPRMergeCount(t *testing.T) {
 	s, gitmock, githubmock, _, output := makeTestObjects(t)
 	assert := require.New(t)