github: apply rebaseMerge config in MergePullRequest implementation

Update the `GitHubInterface.MergePullRequest` interface to take a `mergeMethod` argument from the repo configuration. This is a workaround for target branches that require signed commits as described in issue #195.
ejoffe · Feb 20, 2022 · 8ed6d1c · 8ed6d1c
1 parent 4fa1d71
commit 8ed6d1c
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 17 deletions.
diff --git a/github/githubclient/client.go b/github/githubclient/client.go
@@ -463,8 +463,12 @@ func (c *client) CommentPullRequest(ctx context.Context, pr *github.PullRequest,
 	}
 }
 
-func (c *client) MergePullRequest(ctx context.Context, pr *github.PullRequest) {
-	log.Debug().Interface("PR", pr).Msg("MergePullRequest")
+func (c *client) MergePullRequest(ctx context.Context,
+	pr *github.PullRequest, mergeMethod githubv4.PullRequestMergeMethod) {
+	log.Debug().
+		Interface("PR", pr).
+		Str("mergeMethod", string(mergeMethod)).
+		Msg("MergePullRequest")
 
 	var mergepr struct {
 		MergePullRequest struct {
@@ -473,7 +477,6 @@ func (c *client) MergePullRequest(ctx context.Context, pr *github.PullRequest) {
 			}
 		} `graphql:"mergePullRequest(input: $input)"`
 	}
-	mergeMethod := githubv4.PullRequestMergeMethodRebase
 	mergePRInput := githubv4.MergePullRequestInput{
 		PullRequestID: pr.ID,
 		MergeMethod:   &mergeMethod,

diff --git a/github/interface.go b/github/interface.go
@@ -4,14 +4,15 @@ import (
 	"context"
 
 	"github.com/ejoffe/spr/git"
+	"github.com/shurcooL/githubv4"
 )
 
 type GitHubInterface interface {
 	GetInfo(ctx context.Context, gitcmd git.GitInterface) *GitHubInfo
 	CreatePullRequest(ctx context.Context, info *GitHubInfo, commit git.Commit, prevCommit *git.Commit) *PullRequest
 	UpdatePullRequest(ctx context.Context, info *GitHubInfo, pr *PullRequest, commit git.Commit, prevCommit *git.Commit)
 	CommentPullRequest(ctx context.Context, pr *PullRequest, comment string)
-	MergePullRequest(ctx context.Context, pr *PullRequest)
+	MergePullRequest(ctx context.Context, pr *PullRequest, mergeMethod githubv4.PullRequestMergeMethod)
 	ClosePullRequest(ctx context.Context, pr *PullRequest)
 }
 

diff --git a/github/mockclient/mockclient.go b/github/mockclient/mockclient.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/ejoffe/spr/git"
 	"github.com/ejoffe/spr/github"
+	"github.com/shurcooL/githubv4"
 	"github.com/stretchr/testify/require"
 )
 
@@ -76,11 +77,13 @@ func (c *MockClient) CommentPullRequest(ctx context.Context, pr *github.PullRequ
 	})
 }
 
-func (c *MockClient) MergePullRequest(ctx context.Context, pr *github.PullRequest) {
-	fmt.Printf("HUB: MergePullRequest\n")
+func (c *MockClient) MergePullRequest(ctx context.Context,
+	pr *github.PullRequest, mergeMethod githubv4.PullRequestMergeMethod) {
+	fmt.Printf("HUB: MergePullRequest, method=%q\n", mergeMethod)
 	c.verifyExpectation(expectation{
-		op:     mergePullRequestOP,
-		commit: pr.Commit,
+		op:          mergePullRequestOP,
+		commit:      pr.Commit,
+		mergeMethod: mergeMethod,
 	})
 }
 
@@ -121,10 +124,11 @@ func (c *MockClient) ExpectCommentPullRequest(commit git.Commit) {
 	})
 }
 
-func (c *MockClient) ExpectMergePullRequest(commit git.Commit) {
+func (c *MockClient) ExpectMergePullRequest(commit git.Commit, mergeMethod githubv4.PullRequestMergeMethod) {
 	c.expect = append(c.expect, expectation{
-		op:     mergePullRequestOP,
-		commit: commit,
+		op:          mergePullRequestOP,
+		commit:      commit,
+		mergeMethod: mergeMethod,
 	})
 }
 
@@ -153,7 +157,8 @@ const (
 )
 
 type expectation struct {
-	op     operation
-	commit git.Commit
-	prev   *git.Commit
+	op          operation
+	commit      git.Commit
+	prev        *git.Commit
+	mergeMethod githubv4.PullRequestMergeMethod
 }
diff --git a/spr/spr.go b/spr/spr.go
@@ -217,7 +217,9 @@ func (sd *stackediff) MergePullRequests(ctx context.Context, upto *int) {
 	sd.profiletimer.Step("MergePullRequests::update pr base")
 
 	// Merge pull request
-	sd.github.MergePullRequest(ctx, prToMerge)
+	mergeMethod, err := sd.config.MergeMethod()
+	check(err)
+	sd.github.MergePullRequest(ctx, prToMerge, mergeMethod)
 	sd.profiletimer.Step("MergePullRequests::merge pr")
 
 	// Close all the pull requests in the stack below the merged pr

diff --git a/spr/spr_test.go b/spr/spr_test.go
@@ -12,6 +12,7 @@ import (
 	"github.com/ejoffe/spr/git/mockgit"
 	"github.com/ejoffe/spr/github"
 	"github.com/ejoffe/spr/github/mockclient"
+	"github.com/shurcooL/githubv4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -24,6 +25,7 @@ func makeTestObjects(t *testing.T) (
 	cfg.Repo.RequireApproval = true
 	cfg.Repo.GitHubRemote = "origin"
 	cfg.Repo.GitHubBranch = "master"
+	cfg.Repo.MergeMethod = "rebase"
 	gitmock = mockgit.NewMockGit(t)
 	githubmock = mockclient.NewMockClient(t)
 	githubmock.Info = &github.GitHubInfo{
@@ -124,7 +126,7 @@ func TestSPRBasicFlowFourCommits(t *testing.T) {
 	// 'git spr -m' :: MergePullRequest :: commits=[a1, a2, a3, a4]
 	githubmock.ExpectGetInfo()
 	githubmock.ExpectUpdatePullRequest(c4, nil)
-	githubmock.ExpectMergePullRequest(c4)
+	githubmock.ExpectMergePullRequest(c4, githubv4.PullRequestMergeMethodRebase)
 	githubmock.ExpectCommentPullRequest(c1)
 	githubmock.ExpectClosePullRequest(c1)
 	githubmock.ExpectCommentPullRequest(c2)
@@ -218,7 +220,7 @@ func TestSPRAmendCommit(t *testing.T) {
 	// 'git spr -m' :: MergePullRequest :: commits=[a1, a2]
 	githubmock.ExpectGetInfo()
 	githubmock.ExpectUpdatePullRequest(c2, nil)
-	githubmock.ExpectMergePullRequest(c2)
+	githubmock.ExpectMergePullRequest(c2, githubv4.PullRequestMergeMethodRebase)
 	githubmock.ExpectCommentPullRequest(c1)
 	githubmock.ExpectClosePullRequest(c1)
 	githubmock.ExpectCommentPullRequest(c2)