From 1e78e94a41a6177fc5c888b3b298ff1a0ae38cd8 Mon Sep 17 00:00:00 2001 From: Eitan Joffe Date: Wed, 7 Jul 2021 15:10:34 -0700 Subject: [PATCH] Split config to user and repo configs commit-id:3b97470d --- .spr.yml | 5 --- cmd/spr/main.go | 35 ++++----------- config/config.go | 82 ++++++++++++++++++++++++++++++++--- git/realgit/realcmd.go | 2 +- github/githubclient/client.go | 16 +++---- github/pullrequest.go | 16 +++---- github/pullrequest_test.go | 4 +- readme.md | 10 +++-- spr/spr.go | 6 +-- spr/spr_test.go | 27 +++++------- 10 files changed, 125 insertions(+), 78 deletions(-) diff --git a/.spr.yml b/.spr.yml index 913ffe6..f38e8a7 100644 --- a/.spr.yml +++ b/.spr.yml @@ -2,8 +2,3 @@ githubRepoOwner: ejoffe githubRepoName: spr requireChecks: true requireApproval: false -showPRLink: true -cleanupRemoteBranch: true -logGitCommands: false -logGitHubCalls: false -statusBitsHeader: false diff --git a/cmd/spr/main.go b/cmd/spr/main.go index e302515..1c0c7a9 100644 --- a/cmd/spr/main.go +++ b/cmd/spr/main.go @@ -28,7 +28,7 @@ func init() { } func main() { - gitcmd := realgit.NewGitCmd(&config.Config{}) + gitcmd := realgit.NewGitCmd(config.DefaultConfig()) // check that we are inside a git dir var output string err := gitcmd.Git("status --porcelain", &output) @@ -38,31 +38,12 @@ func main() { os.Exit(2) } - // parse configuration - cfg := config.Config{} - configfilepath := config.ConfigFilePath(gitcmd) - rake.LoadSources(&cfg, - rake.DefaultSource(), - config.GitHubRemoteSource(&cfg, gitcmd), - rake.YamlFileSource(configfilepath), - rake.YamlFileWriter(configfilepath), - ) - - if cfg.GitHubRepoOwner == "" { - fmt.Println("unable to auto configure repository owner - must be set manually in .spr.yml") - os.Exit(3) - } - - if cfg.GitHubRepoName == "" { - fmt.Println("unable to auto configure repository name - must be set manually in .spr.yml") - os.Exit(4) - } - - gitcmd = realgit.NewGitCmd(&cfg) + cfg := config.ParseConfig(gitcmd) + gitcmd = realgit.NewGitCmd(cfg) ctx := context.Background() - client := githubclient.NewGitHubClient(ctx, &cfg) - stackedpr := spr.NewStackedPR(&cfg, client, gitcmd, os.Stdout) + client := githubclient.NewGitHubClient(ctx, cfg) + stackedpr := spr.NewStackedPR(cfg, client, gitcmd, os.Stdout) detailFlag := &cli.BoolFlag{ Name: "detail", @@ -127,12 +108,12 @@ VERSION: {{.Version}} if c.IsSet("profile") { stackedpr.ProfilingEnable() } - if c.IsSet("detail") || cfg.StatusBitsHeader { + if c.IsSet("detail") || cfg.User.StatusBitsHeader { stackedpr.DetailEnabled = true } if c.IsSet("verbose") { - cfg.LogGitCommands = true - cfg.LogGitHubCalls = true + cfg.User.LogGitCommands = true + cfg.User.LogGitHubCalls = true } return nil }, diff --git a/config/config.go b/config/config.go index d4a0e77..7b36948 100644 --- a/config/config.go +++ b/config/config.go @@ -2,34 +2,104 @@ package config import ( "fmt" + "os" "path" "path/filepath" "regexp" "strings" + "github.com/ejoffe/rake" "github.com/ejoffe/spr/git" ) -// Config object to hold spr configuration type Config struct { + Repo *RepoConfig + User *UserConfig +} + +// Config object to hold spr configuration +type RepoConfig struct { GitHubRepoOwner string `yaml:"githubRepoOwner"` GitHubRepoName string `yaml:"githubRepoName"` - RequireChecks bool `default:"true" yaml:"requireChecks"` - RequireApproval bool `default:"true" yaml:"requireApproval"` + RequireChecks bool `default:"true" yaml:"requireChecks"` + RequireApproval bool `default:"true" yaml:"requireApproval"` +} + +type UserConfig struct { ShowPRLink bool `default:"true" yaml:"showPRLink"` CleanupRemoteBranch bool `default:"true" yaml:"cleanupRemoteBranch"` LogGitCommands bool `default:"false" yaml:"logGitCommands"` LogGitHubCalls bool `default:"false" yaml:"logGitHubCalls"` StatusBitsHeader bool `default:"true" yaml:"statusBitsHeader"` + + Stargazer bool `default:"false" yaml:"stargazer"` + RunCount int `default:"0" yaml:"runcount"` } -func ConfigFilePath(gitcmd git.GitInterface) string { +func EmptyConfig() *Config { + return &Config{ + Repo: &RepoConfig{}, + User: &UserConfig{}, + } +} + +func DefaultConfig() *Config { + cfg := EmptyConfig() + rake.LoadSources(cfg.Repo, + rake.DefaultSource(), + ) + rake.LoadSources(cfg.User, + rake.DefaultSource(), + ) + return cfg +} + +func ParseConfig(gitcmd git.GitInterface) *Config { + cfg := EmptyConfig() + + rake.LoadSources(cfg.Repo, + rake.DefaultSource(), + GitHubRemoteSource(cfg, gitcmd), + rake.YamlFileSource(RepoConfigFilePath(gitcmd)), + rake.YamlFileWriter(RepoConfigFilePath(gitcmd)), + ) + + if cfg.Repo.GitHubRepoOwner == "" { + fmt.Println("unable to auto configure repository owner - must be set manually in .spr.yml") + os.Exit(3) + } + + if cfg.Repo.GitHubRepoName == "" { + fmt.Println("unable to auto configure repository name - must be set manually in .spr.yml") + os.Exit(4) + } + + rake.LoadSources(cfg.User, + rake.DefaultSource(), + rake.YamlFileSource(UserConfigFilePath()), + ) + + cfg.User.RunCount = cfg.User.RunCount + 1 + rake.LoadSources(cfg.User, + rake.YamlFileWriter(UserConfigFilePath())) + + return cfg +} + +func RepoConfigFilePath(gitcmd git.GitInterface) string { rootdir := gitcmd.RootDir() filepath := filepath.Clean(path.Join(rootdir, ".spr.yml")) return filepath } +func UserConfigFilePath() string { + rootdir, err := os.UserHomeDir() + check(err) + filepath := filepath.Clean(path.Join(rootdir, ".spr.yml")) + return filepath +} + func GitHubRemoteSource(config *Config, gitcmd git.GitInterface) *remoteSource { return &remoteSource{ gitcmd: gitcmd, @@ -51,8 +121,8 @@ func (s *remoteSource) Load(_ interface{}) { for _, line := range lines { repoOwner, repoName, match := getRepoDetailsFromRemote(line) if match { - s.config.GitHubRepoOwner = repoOwner - s.config.GitHubRepoName = repoName + s.config.Repo.GitHubRepoOwner = repoOwner + s.config.Repo.GitHubRepoName = repoName break } } diff --git a/git/realgit/realcmd.go b/git/realgit/realcmd.go index 30a7b8d..85f41b7 100644 --- a/git/realgit/realcmd.go +++ b/git/realgit/realcmd.go @@ -38,7 +38,7 @@ func (c *gitcmd) Git(argStr string, output *string) error { // if output is not nil it will be set to the output of the command log.Debug().Msg("git " + argStr) - if c.config.LogGitCommands { + if c.config.User.LogGitCommands { fmt.Printf("> git %s\n", argStr) } args := strings.Split(argStr, " ") diff --git a/github/githubclient/client.go b/github/githubclient/client.go index 6c1dbaf..61d1d70 100644 --- a/github/githubclient/client.go +++ b/github/githubclient/client.go @@ -42,7 +42,7 @@ type client struct { var pullRequestRegex = regexp.MustCompile(`pr/[a-zA-Z0-9_\-]+/([a-zA-Z0-9_\-/]+)/([a-f0-9]{8})$`) func (c *client) GetInfo(ctx context.Context, gitcmd git.GitInterface) *github.GitHubInfo { - if c.config.LogGitHubCalls { + if c.config.User.LogGitHubCalls { fmt.Printf("> github fetch pull requests\n") } var query struct { @@ -80,8 +80,8 @@ func (c *client) GetInfo(ctx context.Context, gitcmd git.GitInterface) *github.G } `graphql:"repository(owner:$repo_owner, name:$repo_name)"` } variables := map[string]interface{}{ - "repo_owner": githubv4.String(c.config.GitHubRepoOwner), - "repo_name": githubv4.String(c.config.GitHubRepoName), + "repo_owner": githubv4.String(c.config.Repo.GitHubRepoOwner), + "repo_name": githubv4.String(c.config.Repo.GitHubRepoName), } err := c.api.Query(ctx, &query, variables) check(err) @@ -191,7 +191,7 @@ func (c *client) CreatePullRequest(ctx context.Context, }, } - if c.config.LogGitHubCalls { + if c.config.User.LogGitHubCalls { fmt.Printf("> github create %d: %s\n", pr.Number, pr.Title) } @@ -201,7 +201,7 @@ func (c *client) CreatePullRequest(ctx context.Context, func (c *client) UpdatePullRequest(ctx context.Context, info *github.GitHubInfo, pr *github.PullRequest, commit git.Commit, prevCommit *git.Commit) { - if c.config.LogGitHubCalls { + if c.config.User.LogGitHubCalls { fmt.Printf("> github update %d - %s\n", pr.Number, pr.Title) } @@ -261,7 +261,7 @@ func (c *client) CommentPullRequest(ctx context.Context, pr *github.PullRequest, Msg("pull request update failed") } - if c.config.LogGitHubCalls { + if c.config.User.LogGitHubCalls { fmt.Printf("> github add comment %d: %s\n", pr.Number, pr.Title) } } @@ -292,7 +292,7 @@ func (c *client) MergePullRequest(ctx context.Context, pr *github.PullRequest) { } check(err) - if c.config.LogGitHubCalls { + if c.config.User.LogGitHubCalls { fmt.Printf("> github merge %d: %s\n", pr.Number, pr.Title) } } @@ -319,7 +319,7 @@ func (c *client) ClosePullRequest(ctx context.Context, pr *github.PullRequest) { Msg("pull request close failed") } - if c.config.LogGitHubCalls { + if c.config.User.LogGitHubCalls { fmt.Printf("> github close %d: %s\n", pr.Number, pr.Title) } } diff --git a/github/pullrequest.go b/github/pullrequest.go index 813af23..a4a3bab 100644 --- a/github/pullrequest.go +++ b/github/pullrequest.go @@ -97,10 +97,10 @@ func (pr *PullRequest) Mergeable(config *config.Config) bool { if !pr.MergeStatus.Stacked { return false } - if config.RequireChecks && pr.MergeStatus.ChecksPass != CheckStatusPass { + if config.Repo.RequireChecks && pr.MergeStatus.ChecksPass != CheckStatusPass { return false } - if config.RequireApproval && !pr.MergeStatus.ReviewApproved { + if config.Repo.RequireApproval && !pr.MergeStatus.ReviewApproved { return false } return true @@ -114,10 +114,10 @@ func (pr *PullRequest) Ready(config *config.Config) bool { if !pr.MergeStatus.NoConflicts { return false } - if config.RequireChecks && pr.MergeStatus.ChecksPass != CheckStatusPass { + if config.Repo.RequireChecks && pr.MergeStatus.ChecksPass != CheckStatusPass { return false } - if config.RequireApproval && !pr.MergeStatus.ReviewApproved { + if config.Repo.RequireApproval && !pr.MergeStatus.ReviewApproved { return false } return true @@ -133,7 +133,7 @@ func (pr *PullRequest) StatusString(config *config.Config) string { statusString += pr.MergeStatus.ChecksPass.String(config) - if config.RequireApproval { + if config.Repo.RequireApproval { if pr.MergeStatus.ReviewApproved { statusString += checkmark } else { @@ -166,9 +166,9 @@ func (pr *PullRequest) String(config *config.Config) string { } prInfo := fmt.Sprintf("%3d", pr.Number) - if config.ShowPRLink { + if config.User.ShowPRLink { prInfo = fmt.Sprintf("github.com/%s/%s/pull/%d", - config.GitHubRepoOwner, config.GitHubRepoName, pr.Number) + config.Repo.GitHubRepoOwner, config.Repo.GitHubRepoName, pr.Number) } line := fmt.Sprintf("%s %s : %s", prStatus, prInfo, pr.Title) @@ -189,7 +189,7 @@ func (pr *PullRequest) String(config *config.Config) string { } func (cs checkStatus) String(config *config.Config) string { - if config.RequireChecks { + if config.Repo.RequireChecks { switch cs { case CheckStatusUnknown: return "?" diff --git a/github/pullrequest_test.go b/github/pullrequest_test.go index aa6b18b..8313c0d 100644 --- a/github/pullrequest_test.go +++ b/github/pullrequest_test.go @@ -25,7 +25,7 @@ func TestSortPullRequests(t *testing.T) { }, } - config := &config.Config{} + config := config.EmptyConfig() prs = SortPullRequests(prs, config) if prs[0].Number != 1 { t.Fatalf("prs not sorted correctly %v\n", prs) @@ -57,7 +57,7 @@ func TestSortPullRequestsMixed(t *testing.T) { }, } - config := &config.Config{} + config := config.EmptyConfig() prs = SortPullRequests(prs, config) if prs[0].Number != 1 { t.Fatalf("prs not sorted correctly %v\n", prs) diff --git a/readme.md b/readme.md index bcbcde1..585487b 100644 --- a/readme.md +++ b/readme.md @@ -139,15 +139,19 @@ Show Current Pull Requests Configuration ------------- -When the script is run for the first time a config file **.spr.yml** is created in your repository base dir. -These are the available configurations: +When the script is run for the first time two config files are created. +Repository configuration is saved to .spr.yml in the repository base directory. +User specific configuration is saved to .spr.yml in the user home directory. -| Name | Type | Default | Description +| Repository Config | Type | Default | Description | | ------------------- | ---- | ------- | -------------------------------------------------------------- | | githubRepoOwner | str | | name of the github owner (fetched from git remote config) | | githubRepoName | str | | name of the github repository (fetched from git remote config) | | requireChecks | bool | true | require checks to pass in order to merge | | requireApproval | bool | true | require pull request approval in order to merge | + +| User Config | Type | Default | Description | +| ------------------- | ---- | ------- | -------------------------------------------------------------- | | showPRLink | bool | true | show full pull request http link | | cleanupRemoteBranch | bool | true | delete remote branch after pull request merges | | logGitCommands | bool | false | logs all git commands to stdout | diff --git a/spr/spr.go b/spr/spr.go index f0d1f07..3a3a253 100644 --- a/spr/spr.go +++ b/spr/spr.go @@ -181,7 +181,7 @@ func (sd *stackediff) MergePullRequests(ctx context.Context) { sd.github.MergePullRequest(ctx, prToMerge) sd.profiletimer.Step("MergePullRequests::merge pr") - if sd.config.CleanupRemoteBranch { + if sd.config.User.CleanupRemoteBranch { sd.gitcmd.Git(fmt.Sprintf("push -d origin %s", prToMerge.FromBranch), nil) } @@ -191,12 +191,12 @@ func (sd *stackediff) MergePullRequests(ctx context.Context) { pr := githubInfo.PullRequests[i] comment := fmt.Sprintf( "commit MERGED in pull request [#%d](https://github.com/%s/%s/pull/%d)", - prToMerge.Number, sd.config.GitHubRepoOwner, sd.config.GitHubRepoName, prToMerge.Number) + prToMerge.Number, sd.config.Repo.GitHubRepoOwner, sd.config.Repo.GitHubRepoName, prToMerge.Number) sd.github.CommentPullRequest(ctx, pr, comment) sd.github.ClosePullRequest(ctx, pr) - if sd.config.CleanupRemoteBranch { + if sd.config.User.CleanupRemoteBranch { sd.gitcmd.Git(fmt.Sprintf("push -d origin %s", pr.FromBranch), nil) } } diff --git a/spr/spr_test.go b/spr/spr_test.go index 71afdb2..68f55bc 100644 --- a/spr/spr_test.go +++ b/spr/spr_test.go @@ -18,10 +18,9 @@ import ( func TestSPRBasicFlowFourCommits(t *testing.T) { assert := require.New(t) - cfg := config.Config{ - RequireChecks: true, - RequireApproval: true, - } + cfg := config.EmptyConfig() + cfg.Repo.RequireChecks = true + cfg.Repo.RequireApproval = true gitmock := mockgit.NewMockGit(t) githubmock := mockclient.NewMockClient(t) githubmock.Info = &github.GitHubInfo{ @@ -30,7 +29,7 @@ func TestSPRBasicFlowFourCommits(t *testing.T) { LocalBranch: "master", } var output bytes.Buffer - s := NewStackedPR(&cfg, githubmock, gitmock, &output) + s := NewStackedPR(cfg, githubmock, gitmock, &output) ctx := context.Background() @@ -126,10 +125,9 @@ func TestSPRBasicFlowFourCommits(t *testing.T) { func TestSPRAmendCommit(t *testing.T) { assert := require.New(t) - cfg := config.Config{ - RequireChecks: true, - RequireApproval: true, - } + cfg := config.EmptyConfig() + cfg.Repo.RequireChecks = true + cfg.Repo.RequireApproval = true gitmock := mockgit.NewMockGit(t) githubmock := mockclient.NewMockClient(t) githubmock.Info = &github.GitHubInfo{ @@ -138,7 +136,7 @@ func TestSPRAmendCommit(t *testing.T) { LocalBranch: "master", } var output bytes.Buffer - s := NewStackedPR(&cfg, githubmock, gitmock, &output) + s := NewStackedPR(cfg, githubmock, gitmock, &output) ctx := context.Background() @@ -226,10 +224,9 @@ func TestSPRAmendCommit(t *testing.T) { func TestSPRReorderCommit(t *testing.T) { assert := require.New(t) - cfg := config.Config{ - RequireChecks: true, - RequireApproval: true, - } + cfg := config.EmptyConfig() + cfg.Repo.RequireChecks = true + cfg.Repo.RequireApproval = true gitmock := mockgit.NewMockGit(t) githubmock := mockclient.NewMockClient(t) githubmock.Info = &github.GitHubInfo{ @@ -238,7 +235,7 @@ func TestSPRReorderCommit(t *testing.T) { LocalBranch: "master", } var output bytes.Buffer - s := NewStackedPR(&cfg, githubmock, gitmock, &output) + s := NewStackedPR(cfg, githubmock, gitmock, &output) ctx := context.Background()