From f62c12f9fae6d39b58d5eda3bd1183e2c0d0ae13 Mon Sep 17 00:00:00 2001 From: Eitan Joffe Date: Thu, 7 Sep 2023 16:40:34 -0700 Subject: [PATCH] Fix remote origin branch config fixes #349 commit-id:f9c27350 --- config/config.go | 23 ++++++---------- config/config_parser/config_parser.go | 6 +---- config/config_parser/remote_branch.go | 8 +++--- config/config_test.go | 11 +++----- git/helpers.go | 6 ++--- github/githubclient/client.go | 6 ++--- readme.md | 38 ++++++++++++++------------- spr/spr.go | 6 ++--- spr/spr_test.go | 4 +-- 9 files changed, 47 insertions(+), 61 deletions(-) diff --git a/config/config.go b/config/config.go index 829de30..e6dd20b 100644 --- a/config/config.go +++ b/config/config.go @@ -9,10 +9,9 @@ import ( ) type Config struct { - Repo *RepoConfig - User *UserConfig - Internal *InternalConfig - State *InternalState + Repo *RepoConfig + User *UserConfig + State *InternalState } // Config object to hold spr configuration @@ -21,6 +20,9 @@ type RepoConfig struct { GitHubRepoName string `yaml:"githubRepoName"` GitHubHost string `default:"github.com" yaml:"githubHost"` + GitHubRemote string `default:"origin" yaml:"githubRemote"` + GitHubBranch string `default:"main" yaml:"githubBranch"` + RequireChecks bool `default:"true" yaml:"requireChecks"` RequireApproval bool `default:"true" yaml:"requireApproval"` @@ -36,11 +38,6 @@ type RepoConfig struct { ForceFetchTags bool `default:"false" yaml:"forceFetchTags"` } -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"` @@ -62,9 +59,8 @@ type InternalState struct { func EmptyConfig() *Config { return &Config{ - Repo: &RepoConfig{}, - User: &UserConfig{}, - Internal: &InternalConfig{}, + Repo: &RepoConfig{}, + User: &UserConfig{}, State: &InternalState{ MergeCheckCommit: map[string]string{}, }, @@ -79,9 +75,6 @@ func DefaultConfig() *Config { rake.LoadSources(cfg.User, rake.DefaultSource(), ) - rake.LoadSources(cfg.Internal, - rake.DefaultSource(), - ) cfg.User.LogGitCommands = false cfg.User.LogGitHubCalls = false diff --git a/config/config_parser/config_parser.go b/config/config_parser/config_parser.go index 1ddd366..59268ae 100644 --- a/config/config_parser/config_parser.go +++ b/config/config_parser/config_parser.go @@ -18,6 +18,7 @@ func ParseConfig(gitcmd git.GitInterface) *config.Config { rake.DefaultSource(), NewGitHubRemoteSource(cfg, gitcmd), rake.YamlFileSource(RepoConfigFilePath(gitcmd)), + NewRemoteBranchSource(gitcmd), ) if cfg.Repo.GitHubHost == "" { fmt.Println("unable to auto configure repository host - must be set manually in .spr.yml") @@ -38,11 +39,6 @@ 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()), diff --git a/config/config_parser/remote_branch.go b/config/config_parser/remote_branch.go index 2a1fad5..45b89f5 100644 --- a/config/config_parser/remote_branch.go +++ b/config/config_parser/remote_branch.go @@ -1,7 +1,6 @@ package config_parser import ( - "fmt" "regexp" "github.com/ejoffe/spr/config" @@ -27,12 +26,11 @@ func (s *remoteBranch) Load(cfg interface{}) { matches := _remoteBranchRegex.FindStringSubmatch(output) if matches == nil { - fmt.Printf("error: unable to fetch remote branch info, using defaults") return } - internalCfg := cfg.(*config.InternalConfig) + repoCfg := cfg.(*config.RepoConfig) - internalCfg.GitHubRemote = matches[2] - internalCfg.GitHubBranch = matches[3] + repoCfg.GitHubRemote = matches[2] + repoCfg.GitHubBranch = matches[3] } diff --git a/config/config_test.go b/config/config_test.go index 27d8d95..eb72225 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -9,9 +9,8 @@ import ( func TestEmptyConfig(t *testing.T) { expect := &Config{ - Repo: &RepoConfig{}, - User: &UserConfig{}, - Internal: &InternalConfig{}, + Repo: &RepoConfig{}, + User: &UserConfig{}, State: &InternalState{ MergeCheckCommit: map[string]string{}, }, @@ -25,6 +24,8 @@ func TestDefaultConfig(t *testing.T) { Repo: &RepoConfig{ GitHubRepoOwner: "", GitHubRepoName: "", + GitHubRemote: "origin", + GitHubBranch: "main", GitHubHost: "github.com", RequireChecks: true, RequireApproval: true, @@ -40,10 +41,6 @@ func TestDefaultConfig(t *testing.T) { StatusBitsHeader: true, StatusBitsEmojis: true, }, - Internal: &InternalConfig{ - GitHubRemote: "origin", - GitHubBranch: "main", - }, State: &InternalState{ MergeCheckCommit: map[string]string{}, }, diff --git a/git/helpers.go b/git/helpers.go index 0b501ae..a52c638 100644 --- a/git/helpers.go +++ b/git/helpers.go @@ -25,7 +25,7 @@ func GetLocalBranchName(gitcmd GitInterface) string { } func BranchNameFromCommit(cfg *config.Config, commit Commit) string { - remoteBranchName := cfg.Internal.GitHubBranch + remoteBranchName := cfg.Repo.GitHubBranch return "spr/" + remoteBranchName + "/" + commit.CommitID } @@ -48,7 +48,7 @@ func GetLocalTopCommit(cfg *config.Config, gitcmd GitInterface) *Commit { func GetLocalCommitStack(cfg *config.Config, gitcmd GitInterface) []Commit { var commitLog string logCommand := fmt.Sprintf("log --format=medium --no-color %s/%s..HEAD", - cfg.Internal.GitHubRemote, cfg.Internal.GitHubBranch) + cfg.Repo.GitHubRemote, cfg.Repo.GitHubBranch) gitcmd.MustGit(logCommand, &commitLog) commits, valid := parseLocalCommitStack(commitLog) if !valid { @@ -56,7 +56,7 @@ func GetLocalCommitStack(cfg *config.Config, gitcmd GitInterface) []Commit { rewordPath, err := exec.LookPath("spr_reword_helper") check(err) rebaseCommand := fmt.Sprintf("rebase %s/%s -i --autosquash --autostash", - cfg.Internal.GitHubRemote, cfg.Internal.GitHubBranch) + cfg.Repo.GitHubRemote, cfg.Repo.GitHubBranch) gitcmd.GitWithEditor(rebaseCommand, nil, rewordPath) gitcmd.MustGit(logCommand, &commitLog) diff --git a/github/githubclient/client.go b/github/githubclient/client.go index bbfd040..a3f0be6 100644 --- a/github/githubclient/client.go +++ b/github/githubclient/client.go @@ -182,7 +182,7 @@ func (c *client) GetInfo(ctx context.Context, gitcmd git.GitInterface) *github.G c.config.Repo.GitHubRepoName) check(err) - targetBranch := c.config.Internal.GitHubBranch + targetBranch := c.config.Repo.GitHubBranch localCommitStack := git.GetLocalCommitStack(c.config, gitcmd) pullRequests := matchPullRequestStack(c.config.Repo, targetBranch, localCommitStack, resp.Viewer.PullRequests) @@ -354,7 +354,7 @@ 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 := c.config.Internal.GitHubBranch + baseRefName := c.config.Repo.GitHubBranch if prevCommit != nil { baseRefName = git.BranchNameFromCommit(c.config, *prevCommit) } @@ -512,7 +512,7 @@ func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface, fmt.Printf("> github update %d : %s\n", pr.Number, pr.Title) } - baseRefName := c.config.Internal.GitHubBranch + baseRefName := c.config.Repo.GitHubBranch if prevCommit != nil { baseRefName = git.BranchNameFromCommit(c.config, *prevCommit) } diff --git a/readme.md b/readme.md index ebb6727..1d19999 100644 --- a/readme.md +++ b/readme.md @@ -151,31 +151,33 @@ User specific configuration is saved to .spr.yml in the user home directory. | Repository Config | Type | Default | Description | |-------------------------| ---- |------------|-----------------------------------------------------------------------------------| -| requireChecks | bool | true | require checks to pass in order to merge | -| 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) | -| githubHost | str | github.com | github host, can be updated for github enterprise use case | -| mergeMethod | str | rebase | merge method, valid values: [rebase, squash, merge] | +| requireChecks | bool | true | require checks to pass in order to merge | +| 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 | | prTemplatePath | str | | path to PR template (e.g. .github/PULL_REQUEST_TEMPLATE/pull_request_template.md) | -| prTemplateInsertStart | str | | text to search for in PR template that determines body insert start location | -| prTemplateInsertEnd | str | | text to search for in PR template that determines body insert end location | +| prTemplateInsertStart | str | | text to search for in PR template that determines body insert start location | +| prTemplateInsertEnd | str | | text to search for in PR template that determines body insert end location | | mergeCheck | str | | enforce a pre-merge check using 'git spr check' | | forceFetchTags | bool | false | also fetch tags when running 'git spr update' | | branchNameIncludeTarget | bool | false | include target branch name in pull request branch name | -| User Config | Type | Default | Description | -| -------------------- | ---- | ------- | ----------------------------------------------------------------- | -| showPRLink | bool | true | show full pull request http link | -| logGitCommands | bool | true | logs all git commands to stdout | -| logGitHubCalls | bool | true | logs all github api calls to stdout | -| statusBitsHeader | bool | true | show status bits type headers | -| statusBitsEmojis | bool | true | show status bits using fancy emojis | -| 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 | +| User Config | Type | Default | Description | +| -------------------- | ---- | ------- | --------------------------------------------------------------- | +| showPRLink | bool | true | show full pull request http link | +| logGitCommands | bool | true | logs all git commands to stdout | +| logGitHubCalls | bool | true | logs all github api calls to stdout | +| statusBitsHeader | bool | true | show status bits type headers | +| statusBitsEmojis | bool | true | show status bits using fancy emojis | +| 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 | Happy Coding! ------------- diff --git a/spr/spr.go b/spr/spr.go index 93e4e3f..b43c8f6 100644 --- a/spr/spr.go +++ b/spr/spr.go @@ -81,7 +81,7 @@ func (sd *stackediff) AmendCommit(ctx context.Context) { sd.gitcmd.MustGit("commit --fixup "+localCommits[commitIndex].CommitHash, nil) rebaseCmd := fmt.Sprintf("rebase -i --autosquash --autostash %s/%s", - sd.config.Internal.GitHubRemote, sd.config.Internal.GitHubBranch) + sd.config.Repo.GitHubRemote, sd.config.Repo.GitHubBranch) sd.gitcmd.MustGit(rebaseCmd, nil) } @@ -482,7 +482,7 @@ func (sd *stackediff) fetchAndGetGitHubInfo(ctx context.Context) *github.GitHubI sd.gitcmd.MustGit("fetch", nil) } rebaseCommand := fmt.Sprintf("rebase %s/%s --autostash", - sd.config.Internal.GitHubRemote, sd.config.Internal.GitHubBranch) + sd.config.Repo.GitHubRemote, sd.config.Repo.GitHubBranch) err := sd.gitcmd.Git(rebaseCommand, nil) if err != nil { return nil @@ -544,7 +544,7 @@ func (sd *stackediff) syncCommitStackToGitHub(ctx context.Context, commit.CommitHash+":refs/heads/"+branchName) } if len(updatedCommits) > 0 { - pushCommand := fmt.Sprintf("push --force --atomic %s ", sd.config.Internal.GitHubRemote) + pushCommand := fmt.Sprintf("push --force --atomic %s ", sd.config.Repo.GitHubRemote) pushCommand += strings.Join(refNames, " ") sd.gitcmd.MustGit(pushCommand, nil) } diff --git a/spr/spr_test.go b/spr/spr_test.go index a929b37..13cde06 100644 --- a/spr/spr_test.go +++ b/spr/spr_test.go @@ -22,8 +22,8 @@ func makeTestObjects(t *testing.T) ( cfg := config.EmptyConfig() cfg.Repo.RequireChecks = true cfg.Repo.RequireApproval = true - cfg.Internal.GitHubRemote = "origin" - cfg.Internal.GitHubBranch = "master" + cfg.Repo.GitHubRemote = "origin" + cfg.Repo.GitHubBranch = "master" cfg.Repo.MergeMethod = "rebase" gitmock = mockgit.NewMockGit(t) githubmock = mockclient.NewMockClient(t)