Skip to content

Commit

Permalink
Adds support for tracking multiple remote branches
Browse files Browse the repository at this point in the history
commit-id:206a5fad
  • Loading branch information
ejoffe committed Jul 25, 2022
1 parent 72d297b commit dd06c0a
Show file tree
Hide file tree
Showing 11 changed files with 149 additions and 121 deletions.
8 changes: 5 additions & 3 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ type RepoConfig struct {
RequireChecks bool `default:"true" yaml:"requireChecks"`
RequireApproval bool `default:"true" yaml:"requireApproval"`

GitHubRemote string `default:"origin" yaml:"githubRemote"`
GitHubBranch string `default:"master" yaml:"githubBranch"`
MergeMethod string `default:"rebase" yaml:"mergeMethod"`
GitHubRemote string `default:"origin" yaml:"githubRemote"`
GitHubBranch string `default:"master" yaml:"githubBranch"`
RemoteBranches []string `yaml:"remoteBranches"`

MergeMethod string `default:"rebase" yaml:"mergeMethod"`
}

type UserConfig struct {
Expand Down
7 changes: 7 additions & 0 deletions git/mockgit/mockgit.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ type responder interface {

func (m *Mock) ExpectFetch() {
m.expect("git fetch")
m.expect("git branch --no-color").respond("* master")
m.expect("git rebase origin/master --autostash")
}

func (m *Mock) ExpectLogAndRespond(commits []*git.Commit) {
m.expect("git branch --no-color").respond("* master")
m.expect("git log --no-color origin/master..HEAD").commitRespond(commits)
}

Expand All @@ -85,9 +87,14 @@ func (m *Mock) ExpectRemote(remote string) {

func (m *Mock) ExpectFixup(commitHash string) {
m.expect("git commit --fixup " + commitHash)
m.expect("git branch --no-color").respond("* master")
m.expect("git rebase -i --autosquash --autostash origin/master")
}

func (m *Mock) ExpectLocalBranch(name string) {
m.expect("git branch --no-color").respond(name)
}

func (m *Mock) expect(cmd string, args ...interface{}) *Mock {
m.expectedCmd = append(m.expectedCmd, fmt.Sprintf(cmd, args...))
m.response = append(m.response, &commitResponse{valid: false})
Expand Down
56 changes: 51 additions & 5 deletions github/githubclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,8 @@ func (c *client) GetInfo(ctx context.Context, gitcmd git.GitInterface) *github.G
}
}

requests = github.SortPullRequests(requests, c.config)
targetBranch := GetRemoteBranchName(gitcmd, c.config.Repo)
requests = sortPullRequests(requests, c.config, targetBranch)

info := &github.GitHubInfo{
UserName: resp.Viewer.Login,
Expand Down Expand Up @@ -277,10 +278,10 @@ func (c *client) GetAssignableUsers(ctx context.Context) []github.RepoAssignee {
return users
}

func (c *client) CreatePullRequest(ctx context.Context,
func (c *client) CreatePullRequest(ctx context.Context, gitcmd git.GitInterface,
info *github.GitHubInfo, commit git.Commit, prevCommit *git.Commit) *github.PullRequest {

baseRefName := c.config.Repo.GitHubBranch
baseRefName := GetRemoteBranchName(gitcmd, c.config.Repo)
if prevCommit != nil {
baseRefName = branchNameFromCommit(info, *prevCommit)
}
Expand Down Expand Up @@ -360,14 +361,14 @@ func addManualMergeNotice(body string) string {
"Do not merge manually using the UI - doing so may have unexpected results.*"
}

func (c *client) UpdatePullRequest(ctx context.Context,
func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface,
info *github.GitHubInfo, pr *github.PullRequest, commit git.Commit, prevCommit *git.Commit) {

if c.config.User.LogGitHubCalls {
fmt.Printf("> github update %d : %s\n", pr.Number, pr.Title)
}

baseRefName := c.config.Repo.GitHubBranch
baseRefName := GetRemoteBranchName(gitcmd, c.config.Repo)
if prevCommit != nil {
baseRefName = branchNameFromCommit(info, *prevCommit)
}
Expand Down Expand Up @@ -496,10 +497,55 @@ func getLocalBranchName(gitcmd git.GitInterface) string {
panic("cannot determine local git branch name")
}

func GetRemoteBranchName(gitcmd git.GitInterface, repoConfig *config.RepoConfig) string {
localBranchName := getLocalBranchName(gitcmd)

for _, remoteBranchName := range repoConfig.RemoteBranches {
if localBranchName == remoteBranchName {
return remoteBranchName
}
}
return repoConfig.GitHubBranch
}

func branchNameFromCommit(info *github.GitHubInfo, commit git.Commit) string {
return "pr/" + info.UserName + "/" + info.LocalBranch + "/" + commit.CommitID
}

// sortPullRequests sorts the pull requests so that the one that is on top of
// the target branch will come first followed by the ones that are stacked on top.
// The stack order is maintained so that multiple pull requests can be merged in
// the correct order.
func sortPullRequests(prs []*github.PullRequest, config *config.Config, targetBranch string) []*github.PullRequest {
swap := func(i int, j int) {
buf := prs[i]
prs[i] = prs[j]
prs[j] = buf
}

j := 0
for i := 0; i < len(prs); i++ {
for j = i; j < len(prs); j++ {
if prs[j].ToBranch == targetBranch {
targetBranch = prs[j].FromBranch
swap(i, j)
break
}
}
}

// update stacked merge status flag
for _, pr := range prs {
if pr.Ready(config) {
pr.MergeStatus.Stacked = true
} else {
break
}
}

return prs
}

func check(err error) {
if err != nil {
msg := err.Error()
Expand Down
65 changes: 65 additions & 0 deletions github/githubclient/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package githubclient
import (
"testing"

"github.com/ejoffe/spr/config"
"github.com/ejoffe/spr/git"
"github.com/ejoffe/spr/github"
)
Expand Down Expand Up @@ -84,3 +85,67 @@ It even includes some **markdown** formatting.
}
}
}

func TestSortPullRequests(t *testing.T) {
prs := []*github.PullRequest{
{
Number: 3,
FromBranch: "third",
ToBranch: "second",
},
{
Number: 2,
FromBranch: "second",
ToBranch: "first",
},
{
Number: 1,
FromBranch: "first",
ToBranch: "master",
},
}

config := config.DefaultConfig()
prs = sortPullRequests(prs, config, "master")
if prs[0].Number != 1 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
if prs[1].Number != 2 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
if prs[2].Number != 3 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
}

func TestSortPullRequestsMixed(t *testing.T) {
prs := []*github.PullRequest{
{
Number: 3,
FromBranch: "third",
ToBranch: "second",
},
{
Number: 1,
FromBranch: "first",
ToBranch: "master",
},
{
Number: 2,
FromBranch: "second",
ToBranch: "first",
},
}

config := config.DefaultConfig()
prs = sortPullRequests(prs, config, "master")
if prs[0].Number != 1 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
if prs[1].Number != 2 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
if prs[2].Number != 3 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
}
4 changes: 2 additions & 2 deletions github/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (
type GitHubInterface interface {
GetInfo(ctx context.Context, gitcmd git.GitInterface) *GitHubInfo
GetAssignableUsers(ctx context.Context) []RepoAssignee
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)
CreatePullRequest(ctx context.Context, gitcmd git.GitInterface, info *GitHubInfo, commit git.Commit, prevCommit *git.Commit) *PullRequest
UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface, info *GitHubInfo, pr *PullRequest, commit git.Commit, prevCommit *git.Commit)
AddReviewers(ctx context.Context, pr *PullRequest, userIDs []string)
CommentPullRequest(ctx context.Context, pr *PullRequest, comment string)
MergePullRequest(ctx context.Context, pr *PullRequest, mergeMethod genclient.PullRequestMergeMethod)
Expand Down
4 changes: 2 additions & 2 deletions github/mockclient/mockclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (c *MockClient) GetAssignableUsers(ctx context.Context) []github.RepoAssign
}
}

func (c *MockClient) CreatePullRequest(ctx context.Context, info *github.GitHubInfo,
func (c *MockClient) CreatePullRequest(ctx context.Context, gitcmd git.GitInterface, info *github.GitHubInfo,
commit git.Commit, prevCommit *git.Commit) *github.PullRequest {
fmt.Printf("HUB: CreatePullRequest\n")
c.verifyExpectation(expectation{
Expand All @@ -78,7 +78,7 @@ func (c *MockClient) CreatePullRequest(ctx context.Context, info *github.GitHubI
}
}

func (c *MockClient) UpdatePullRequest(ctx context.Context, info *github.GitHubInfo,
func (c *MockClient) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface, info *github.GitHubInfo,
pr *github.PullRequest, commit git.Commit, prevCommit *git.Commit) {
fmt.Printf("HUB: UpdatePullRequest\n")
c.verifyExpectation(expectation{
Expand Down
36 changes: 0 additions & 36 deletions github/pullrequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,42 +53,6 @@ type PullRequestMergeStatus struct {
Stacked bool
}

// SortPullRequests sorts the pull requests so that the one that is on top of
// the target branch will come first followed by the ones that are stacked on top.
// The stack order is maintained so that multiple pull requests can be merged in
// the correct order.
func SortPullRequests(prs []*PullRequest, config *config.Config) []*PullRequest {

swap := func(i int, j int) {
buf := prs[i]
prs[i] = prs[j]
prs[j] = buf
}

targetBranch := config.Repo.GitHubBranch
j := 0
for i := 0; i < len(prs); i++ {
for j = i; j < len(prs); j++ {
if prs[j].ToBranch == targetBranch {
targetBranch = prs[j].FromBranch
swap(i, j)
break
}
}
}

// update stacked merge status flag
for _, pr := range prs {
if pr.Ready(config) {
pr.MergeStatus.Stacked = true
} else {
break
}
}

return prs
}

// Mergeable returns true if the pull request is mergable
func (pr *PullRequest) Mergeable(config *config.Config) bool {
if !pr.MergeStatus.NoConflicts {
Expand Down
64 changes: 0 additions & 64 deletions github/pullrequest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,70 +9,6 @@ import (
"github.com/stretchr/testify/assert"
)

func TestSortPullRequests(t *testing.T) {
prs := []*PullRequest{
{
Number: 3,
FromBranch: "third",
ToBranch: "second",
},
{
Number: 2,
FromBranch: "second",
ToBranch: "first",
},
{
Number: 1,
FromBranch: "first",
ToBranch: "master",
},
}

config := config.DefaultConfig()
prs = SortPullRequests(prs, config)
if prs[0].Number != 1 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
if prs[1].Number != 2 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
if prs[2].Number != 3 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
}

func TestSortPullRequestsMixed(t *testing.T) {
prs := []*PullRequest{
{
Number: 3,
FromBranch: "third",
ToBranch: "second",
},
{
Number: 1,
FromBranch: "first",
ToBranch: "master",
},
{
Number: 2,
FromBranch: "second",
ToBranch: "first",
},
}

config := config.DefaultConfig()
prs = SortPullRequests(prs, config)
if prs[0].Number != 1 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
if prs[1].Number != 2 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
if prs[2].Number != 3 {
t.Fatalf("prs not sorted correctly %v\n", prs)
}
}

func TestMergable(t *testing.T) {
type testcase struct {
pr *PullRequest
Expand Down
7 changes: 5 additions & 2 deletions hook/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/ejoffe/spr/config"
"github.com/ejoffe/spr/git"
"github.com/ejoffe/spr/github/githubclient"
"github.com/rs/zerolog/log"
)

Expand Down Expand Up @@ -44,8 +45,9 @@ func InstallCommitHook(cfg *config.Config, gitcmd git.GitInterface) {
// amend commit stack to add commit-id
rewordPath, err := exec.LookPath("spr_reword_helper")
check(err)
targetBranch := githubclient.GetRemoteBranchName(gitcmd, cfg.Repo)
rebaseCommand := fmt.Sprintf("rebase %s/%s -i --autosquash --autostash",
cfg.Repo.GitHubRemote, cfg.Repo.GitHubBranch)
cfg.Repo.GitHubRemote, targetBranch)
gitcmd.GitWithEditor(rebaseCommand, nil, rewordPath)
} else {
binPath, err := exec.LookPath("spr_commit_hook")
Expand All @@ -56,8 +58,9 @@ func InstallCommitHook(cfg *config.Config, gitcmd git.GitInterface) {
// amend commit stack to add commit-id
rewordPath, err := exec.LookPath("spr_reword_helper")
check(err)
targetBranch := githubclient.GetRemoteBranchName(gitcmd, cfg.Repo)
rebaseCommand := fmt.Sprintf("rebase %s/%s -i --autosquash --autostash",
cfg.Repo.GitHubRemote, cfg.Repo.GitHubBranch)
cfg.Repo.GitHubRemote, targetBranch)
gitcmd.GitWithEditor(rebaseCommand, nil, rewordPath)
}
}

0 comments on commit dd06c0a

Please sign in to comment.