Skip to content

Commit

Permalink
Remove GitHubBranch and GitHubOrigin from config - now fetched automa…
Browse files Browse the repository at this point in the history
…tically using git status

fixes #320

commit-id:3f943623
  • Loading branch information
ejoffe committed Jun 1, 2023
1 parent 1d6b6a4 commit 5c50650
Show file tree
Hide file tree
Showing 12 changed files with 99 additions and 75 deletions.
30 changes: 15 additions & 15 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import (
)

type Config struct {
Repo *RepoConfig
User *UserConfig
State *InternalState
Repo *RepoConfig
User *UserConfig
Internal *InternalConfig
State *InternalState
}

// Config object to hold spr configuration
Expand All @@ -23,10 +24,6 @@ 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"`
RemoteBranches []string `yaml:"remoteBranches"`

MergeMethod string `default:"rebase" yaml:"mergeMethod"`
MergeQueue bool `default:"false" yaml:"mergeQueue"`

Expand All @@ -41,6 +38,11 @@ type RepoConfig struct {
BranchNameIncludeTarget bool `default:"false" yaml:"branchNameIncludeTarget"`
}

type InternalConfig struct {
GitHubRemote string `default:"origin" yaml:"githubRemote"`
GitHubBranch string `default:"main" yaml:"githubBranch"`
}

type UserConfig struct {
ShowPRLink bool `default:"true" yaml:"showPRLink"`
LogGitCommands bool `default:"true" yaml:"logGitCommands"`
Expand All @@ -62,8 +64,9 @@ type InternalState struct {

func EmptyConfig() *Config {
return &Config{
Repo: &RepoConfig{},
User: &UserConfig{},
Repo: &RepoConfig{},
User: &UserConfig{},
Internal: &InternalConfig{},
State: &InternalState{
MergeCheckCommit: map[string]string{},
},
Expand All @@ -78,6 +81,9 @@ func DefaultConfig() *Config {
rake.LoadSources(cfg.User,
rake.DefaultSource(),
)
rake.LoadSources(cfg.Internal,
rake.DefaultSource(),
)

cfg.User.LogGitCommands = false
cfg.User.LogGitHubCalls = false
Expand All @@ -102,9 +108,3 @@ func (c Config) MergeMethod() (genclient.PullRequestMergeMethod, error) {
}
return mergeMethod, err
}

func check(err error) {
if err != nil {
panic(err)
}
}
14 changes: 6 additions & 8 deletions config/config_parser/config_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func ParseConfig(gitcmd git.GitInterface) *config.Config {

rake.LoadSources(cfg.Repo,
rake.DefaultSource(),
GitHubRemoteSource(cfg, gitcmd),
NewGitHubRemoteSource(cfg, gitcmd),
rake.YamlFileSource(RepoConfigFilePath(gitcmd)),
rake.YamlFileWriter(RepoConfigFilePath(gitcmd)),
)
Expand All @@ -39,6 +39,11 @@ func ParseConfig(gitcmd git.GitInterface) *config.Config {
rake.YamlFileSource(UserConfigFilePath()),
)

rake.LoadSources(cfg.Internal,
rake.DefaultSource(),
NewRemoteBranchSource(gitcmd),
)

rake.LoadSources(cfg.State,
rake.DefaultSource(),
rake.YamlFileSource(InternalConfigFilePath()),
Expand Down Expand Up @@ -74,10 +79,3 @@ func InternalConfigFilePath() string {
filepath := filepath.Clean(path.Join(rootdir, ".spr.state"))
return filepath
}

func GitHubRemoteSource(config *config.Config, gitcmd git.GitInterface) *remoteSource {
return &remoteSource{
gitcmd: gitcmd,
config: config,
}
}
4 changes: 1 addition & 3 deletions config/config_parser/config_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ func TestGitHubRemoteSource(t *testing.T) {
GitHubHost: "github.com",
RequireChecks: false,
RequireApproval: false,
GitHubRemote: "",
GitHubBranch: "",
MergeMethod: "",
},
User: &config.UserConfig{
Expand All @@ -81,7 +79,7 @@ func TestGitHubRemoteSource(t *testing.T) {
Repo: &config.RepoConfig{},
User: &config.UserConfig{},
}
source := GitHubRemoteSource(&actual, mock)
source := NewGitHubRemoteSource(&actual, mock)
source.Load(nil)
assert.Equal(t, expect, actual)
}
38 changes: 38 additions & 0 deletions config/config_parser/remote_branch.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package config_parser

import (
"fmt"
"regexp"

"github.com/ejoffe/spr/config"
"github.com/ejoffe/spr/git"
)

type remoteBranch struct {
gitcmd git.GitInterface
}

func NewRemoteBranchSource(gitcmd git.GitInterface) *remoteBranch {
return &remoteBranch{
gitcmd: gitcmd,
}
}

var _remoteBranchRegex = regexp.MustCompile(`^## ([a-zA-Z0-9_\-/\.]+)\.\.\.([a-zA-Z0-9_\-/\.]+)/([a-zA-Z0-9_\-/\.]+)`)

func (s *remoteBranch) Load(cfg interface{}) {
var output string
err := s.gitcmd.Git("status -b --porcelain -u no", &output)
check(err)

matches := _remoteBranchRegex.FindStringSubmatch(output)
if matches == nil {
fmt.Printf("error: unable to fetch remote branch info, using defaults")
return
}

internalCfg := cfg.(*config.InternalConfig)

internalCfg.GitHubRemote = matches[2]
internalCfg.GitHubBranch = matches[3]
}
7 changes: 7 additions & 0 deletions config/config_parser/remote_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ type remoteSource struct {
config *config.Config
}

func NewGitHubRemoteSource(config *config.Config, gitcmd git.GitInterface) *remoteSource {
return &remoteSource{
gitcmd: gitcmd,
config: config,
}
}

func (s *remoteSource) Load(_ interface{}) {
var output string
err := s.gitcmd.Git("remote -v", &output)
Expand Down
11 changes: 7 additions & 4 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import (

func TestEmptyConfig(t *testing.T) {
expect := &Config{
Repo: &RepoConfig{},
User: &UserConfig{},
Repo: &RepoConfig{},
User: &UserConfig{},
Internal: &InternalConfig{},
State: &InternalState{
MergeCheckCommit: map[string]string{},
},
Expand All @@ -27,8 +28,6 @@ func TestDefaultConfig(t *testing.T) {
GitHubHost: "github.com",
RequireChecks: true,
RequireApproval: true,
GitHubRemote: "origin",
GitHubBranch: "master",
MergeMethod: "rebase",
PRTemplatePath: "",
PRTemplateInsertStart: "",
Expand All @@ -41,6 +40,10 @@ func TestDefaultConfig(t *testing.T) {
StatusBitsHeader: true,
StatusBitsEmojis: true,
},
Internal: &InternalConfig{
GitHubRemote: "origin",
GitHubBranch: "main",
},
State: &InternalState{
MergeCheckCommit: map[string]string{},
},
Expand Down
29 changes: 8 additions & 21 deletions git/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ func GetLocalBranchName(gitcmd GitInterface) string {
panic("cannot determine local git branch name")
}

func BranchNameFromCommit(repoConfig *config.RepoConfig, gitcmd GitInterface, commit Commit) string {
if repoConfig.BranchNameIncludeTarget {
remoteBranchName := GetRemoteBranchName(repoConfig, gitcmd)
func BranchNameFromCommit(cfg *config.Config, gitcmd GitInterface, commit Commit) string {
if cfg.Repo.BranchNameIncludeTarget {
remoteBranchName := cfg.Internal.GitHubBranch
return "spr/" + remoteBranchName + "/" + commit.CommitID
}

Expand All @@ -44,23 +44,11 @@ func BranchNameRegex(repoConfig *config.RepoConfig) *regexp.Regexp {
return _branchNameRegex
}

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

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

// GetLocalTopCommit returns the top unmerged commit in the stack
//
// return nil if there are no unmerged commits in the stack
func GetLocalTopCommit(repoConfig *config.RepoConfig, gitcmd GitInterface) *Commit {
commits := GetLocalCommitStack(repoConfig, gitcmd)
func GetLocalTopCommit(cfg *config.Config, gitcmd GitInterface) *Commit {
commits := GetLocalCommitStack(cfg, gitcmd)
if len(commits) == 0 {
return nil
}
Expand All @@ -70,19 +58,18 @@ func GetLocalTopCommit(repoConfig *config.RepoConfig, gitcmd GitInterface) *Comm
// GetLocalCommitStack returns a list of unmerged commits
//
// the list is ordered with the bottom commit in the stack first
func GetLocalCommitStack(repoConfig *config.RepoConfig, gitcmd GitInterface) []Commit {
func GetLocalCommitStack(cfg *config.Config, gitcmd GitInterface) []Commit {
var commitLog string
targetBranch := GetRemoteBranchName(repoConfig, gitcmd)
logCommand := fmt.Sprintf("log --format=medium --no-color %s/%s..HEAD",
repoConfig.GitHubRemote, targetBranch)
cfg.Internal.GitHubRemote, cfg.Internal.GitHubBranch)
gitcmd.MustGit(logCommand, &commitLog)
commits, valid := parseLocalCommitStack(commitLog)
if !valid {
// if not valid - run rebase to add commit ids
rewordPath, err := exec.LookPath("spr_reword_helper")
check(err)
rebaseCommand := fmt.Sprintf("rebase %s/%s -i --autosquash --autostash",
repoConfig.GitHubRemote, targetBranch)
cfg.Internal.GitHubRemote, cfg.Internal.GitHubBranch)
gitcmd.GitWithEditor(rebaseCommand, nil, rewordPath)

gitcmd.MustGit(logCommand, &commitLog)
Expand Down
3 changes: 0 additions & 3 deletions git/mockgit/mockgit.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,10 @@ 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 --format=medium --no-color origin/master..HEAD").commitRespond(commits)
}

Expand All @@ -94,7 +92,6 @@ 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")
}

Expand Down
14 changes: 7 additions & 7 deletions github/githubclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ func (c *client) GetInfo(ctx context.Context, gitcmd git.GitInterface) *github.G
c.config.Repo.GitHubRepoName)
check(err)

targetBranch := git.GetRemoteBranchName(c.config.Repo, gitcmd)
localCommitStack := git.GetLocalCommitStack(c.config.Repo, gitcmd)
targetBranch := c.config.Internal.GitHubBranch
localCommitStack := git.GetLocalCommitStack(c.config, gitcmd)

pullRequests := matchPullRequestStack(c.config.Repo, targetBranch, localCommitStack, resp.Repository.PullRequests)
for _, pr := range pullRequests {
Expand Down Expand Up @@ -340,11 +340,11 @@ func (c *client) GetAssignableUsers(ctx context.Context) []github.RepoAssignee {
func (c *client) CreatePullRequest(ctx context.Context, gitcmd git.GitInterface,
info *github.GitHubInfo, commit git.Commit, prevCommit *git.Commit) *github.PullRequest {

baseRefName := git.GetRemoteBranchName(c.config.Repo, gitcmd)
baseRefName := c.config.Internal.GitHubBranch
if prevCommit != nil {
baseRefName = git.BranchNameFromCommit(c.config.Repo, gitcmd, *prevCommit)
baseRefName = git.BranchNameFromCommit(c.config, gitcmd, *prevCommit)
}
headRefName := git.BranchNameFromCommit(c.config.Repo, gitcmd, commit)
headRefName := git.BranchNameFromCommit(c.config, gitcmd, commit)

log.Debug().Interface("Commit", commit).
Str("FromBranch", headRefName).Str("ToBranch", baseRefName).
Expand Down Expand Up @@ -498,9 +498,9 @@ func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface,
fmt.Printf("> github update %d : %s\n", pr.Number, pr.Title)
}

baseRefName := git.GetRemoteBranchName(c.config.Repo, gitcmd)
baseRefName := c.config.Internal.GitHubBranch
if prevCommit != nil {
baseRefName = git.BranchNameFromCommit(c.config.Repo, gitcmd, *prevCommit)
baseRefName = git.BranchNameFromCommit(c.config, gitcmd, *prevCommit)
}

log.Debug().Interface("Commit", commit).
Expand Down
2 changes: 0 additions & 2 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,6 @@ User specific configuration is saved to .spr.yml in the user home directory.
| requireApproval | bool | true | require pull request approval in order to merge |
| githubRepoOwner | str | | name of the github owner (fetched from git remote config) |
| githubRepoName | str | | name of the github repository (fetched from git remote config) |
| githubRemote | str | origin | github remote name to use |
| githubBranch | str | master | github branch for pull request target |
| githubHost | str | github.com | github host, can be updated for github enterprise use case |
| mergeMethod | str | rebase | merge method, valid values: [rebase, squash, merge] |
| mergeQueue | bool | false | use GitHub merge queue to merge pull requests |
Expand Down
Loading

0 comments on commit 5c50650

Please sign in to comment.