diff --git a/cmd/spr/main.go b/cmd/spr/main.go index 1854d4a5..0b8f4ba9 100644 --- a/cmd/spr/main.go +++ b/cmd/spr/main.go @@ -25,6 +25,13 @@ var ( date = "unknown" ) +func truncate(s string, n int) string { + if len(s) <= n { + return s + } + return s[:n] +} + func init() { zerolog.SetGlobalLevel(zerolog.InfoLevel) log.Logger = log.With().Caller().Logger().Output(zerolog.ConsoleWriter{Out: os.Stderr}) @@ -98,6 +105,13 @@ func main() { Usage: "Show detailed status bits output", } + textFlag := &cli.BoolFlag{ + Name: "text", + Aliases: []string{"t"}, + Value: false, + Usage: "Show plain text output (URL : title)", + } + cli.AppHelpTemplate = `NAME: {{.Name}} - {{.Usage}} @@ -117,7 +131,7 @@ VERSION: fork of {{.Version}} Name: "spr", Usage: "Stacked Pull Requests on GitHub", HideVersion: true, - Version: fmt.Sprintf("%s : %s : %s\n", version, date, commit[:8]), + Version: fmt.Sprintf("%s : %s : %s\n", version, date, truncate(commit, 8)), EnableBashCompletion: true, Authors: []*cli.Author{ { @@ -170,12 +184,16 @@ VERSION: fork of {{.Version}} Name: "status", Aliases: []string{"s", "st"}, Usage: "Show status of open pull requests", - Action: func(c *cli.Context) error { - stackedpr.StatusPullRequests(ctx) - return nil - }, + Action: func(c *cli.Context) error { + if c.IsSet("text") { + stackedpr.TextEnabled = true + } + stackedpr.StatusPullRequests(ctx) + return nil + }, Flags: []cli.Flag{ detailFlag, + textFlag, }, }, { diff --git a/config/config.go b/config/config.go index 10b7f9d0..a3de0976 100644 --- a/config/config.go +++ b/config/config.go @@ -24,6 +24,7 @@ type RepoConfig struct { GitHubBranch string `default:"main" yaml:"githubBranch"` RequireChecks bool `default:"true" yaml:"requireChecks"` + RequiredChecks []string `yaml:"requiredChecks"` RequireApproval bool `default:"true" yaml:"requireApproval"` DefaultReviewers []string `yaml:"defaultReviewers"` diff --git a/git/mockgit/mockgit.go b/git/mockgit/mockgit.go index 0e4e4ef0..dadc95ec 100644 --- a/git/mockgit/mockgit.go +++ b/git/mockgit/mockgit.go @@ -107,21 +107,21 @@ func (m *Mock) ExpectEditStart() { // ExpectEditDoneAmend expects the amend + rebase continue sequence for a successful edit --done func (m *Mock) ExpectEditDoneAmend() { - m.expect("git add -A") + m.expect("git add -u") m.expect("git commit --amend --no-edit") m.expect("git rebase --continue") } // ExpectEditDoneAmendWithConflict expects amend succeeds but rebase --continue fails (conflict) func (m *Mock) ExpectEditDoneAmendWithConflict() { - m.expect("git add -A") + m.expect("git add -u") m.expect("git commit --amend --no-edit") m.expectError("git rebase --continue", errors.New("conflict")) } // ExpectEditDoneConflictResolved expects the conflict resolution path (no amend, just rebase continue) func (m *Mock) ExpectEditDoneConflictResolved() { - m.expect("git add -A") + m.expect("git add -u") m.expect("git rebase --continue") } diff --git a/github/githubclient/client.go b/github/githubclient/client.go index 829d8baa..ce6bd054 100644 --- a/github/githubclient/client.go +++ b/github/githubclient/client.go @@ -1,8 +1,11 @@ package githubclient import ( + "bytes" "context" + "encoding/json" "fmt" + "net/http" "net/url" "os" "path" @@ -152,8 +155,9 @@ func NewGitHubClient(ctx context.Context, config *config.Config) *client { tc := oauth2.NewClient(ctx, ts) var api genclient.Client + var endpoint string if strings.HasSuffix(config.Repo.GitHubHost, "github.com") { - api = genclient.NewClient("https://api.github.com/graphql", tc) + endpoint = "https://api.github.com/graphql" } else { var scheme, host string gitHubRemoteUrl, err := url.Parse(config.Repo.GitHubHost) @@ -165,17 +169,22 @@ func NewGitHubClient(ctx context.Context, config *config.Config) *client { host = gitHubRemoteUrl.Host scheme = gitHubRemoteUrl.Scheme } - api = genclient.NewClient(fmt.Sprintf("%s://%s/api/graphql", scheme, host), tc) + endpoint = fmt.Sprintf("%s://%s/api/graphql", scheme, host) } + api = genclient.NewClient(endpoint, tc) return &client{ - config: config, - api: api, + config: config, + api: api, + graphqlEndpoint: endpoint, + httpClient: tc, } } type client struct { - config *config.Config - api genclient.Client + config *config.Config + api genclient.Client + graphqlEndpoint string + httpClient *http.Client } func (c *client) GetInfo(ctx context.Context, gitcmd git.GitInterface) *github.GitHubInfo { @@ -208,6 +217,22 @@ func (c *client) GetInfo(ctx context.Context, gitcmd git.GitInterface) *github.G localCommitStack := git.GetLocalCommitStack(c.config, gitcmd) pullRequests := matchPullRequestStack(c.config.Repo, c.config.User.BranchPrefix, targetBranch, localCommitStack, pullRequestConnection) + + // When RequiredChecks is explicitly configured, fetch individual check contexts + // and only evaluate the listed checks. This allows non-required check failures + // to be ignored. When RequiredChecks is not set, the statusCheckRollup.state + // from the fezzik query is used as-is (all checks matter). + if c.config.Repo.RequireChecks && len(c.config.Repo.RequiredChecks) > 0 && len(pullRequests) > 0 { + requiredStatus := c.fetchRequiredChecksStatus(ctx, pullRequests) + if requiredStatus != nil { + for _, pr := range pullRequests { + if status, ok := requiredStatus[pr.Number]; ok { + pr.MergeStatus.ChecksPass = status + } + } + } + } + for _, pr := range pullRequests { if pr.Ready(c.config) { pr.MergeStatus.Stacked = true @@ -592,6 +617,231 @@ func (c *client) ClosePullRequest(ctx context.Context, pr *github.PullRequest) { } } +// Response types for the raw GraphQL query that fetches individual check contexts. +// These are used instead of fezzik-generated types because fezzik does not support +// inline fragments on union types (StatusCheckRollupContext = CheckRun | StatusContext). + +type checkContextNode struct { + TypeName string `json:"__typename"` + Name string `json:"name"` // CheckRun + Conclusion *string `json:"conclusion"` // CheckRun (nil when not completed) + Status string `json:"status"` // CheckRun: COMPLETED, IN_PROGRESS, QUEUED, etc. + Context string `json:"context"` // StatusContext + State string `json:"state"` // StatusContext: SUCCESS, FAILURE, PENDING, etc. +} + +type checkContextsResult struct { + Number int `json:"number"` + Commits struct { + Nodes []struct { + Commit struct { + StatusCheckRollup *struct { + Contexts struct { + Nodes []checkContextNode `json:"nodes"` + } `json:"contexts"` + } `json:"statusCheckRollup"` + } `json:"commit"` + } `json:"nodes"` + } `json:"commits"` +} + +type graphqlRequest struct { + Query string `json:"query"` +} + +type graphqlResponse struct { + Data map[string]json.RawMessage `json:"data"` + Errors []struct { + Message string `json:"message"` + } `json:"errors"` +} + +// fetchRequiredChecksStatus makes a single batched GraphQL query to fetch +// individual check contexts for all given pull requests. It evaluates only +// the checks listed in config.Repo.RequiredChecks and returns a map from +// PR number to the computed status. +func (c *client) fetchRequiredChecksStatus(ctx context.Context, pullRequests []*github.PullRequest) map[int]github.CheckStatus { + if len(pullRequests) == 0 { + return nil + } + + if c.config.User.LogGitHubCalls { + fmt.Printf("> github fetch required check status\n") + } + + // Build a single GraphQL query with one aliased field per PR. + var queryBuilder strings.Builder + queryBuilder.WriteString("query {") + for _, pr := range pullRequests { + fmt.Fprintf(&queryBuilder, ` + pr_%d: node(id: %q) { + ... on PullRequest { + number + commits(last: 1) { + nodes { + commit { + statusCheckRollup { + contexts(first: 100) { + nodes { + __typename + ... on CheckRun { + name + conclusion + status + } + ... on StatusContext { + context + state + } + } + } + } + } + } + } + } + }`, pr.Number, pr.ID) + } + queryBuilder.WriteString("\n}") + + reqBody, err := json.Marshal(graphqlRequest{ + Query: queryBuilder.String(), + }) + if err != nil { + log.Warn().Err(err).Msg("failed to marshal required checks query") + return nil + } + + httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, c.graphqlEndpoint, bytes.NewReader(reqBody)) + if err != nil { + log.Warn().Err(err).Msg("failed to create required checks request") + return nil + } + httpReq.Header.Set("Content-Type", "application/json; charset=utf-8") + httpReq.Header.Set("Accept", "application/json; charset=utf-8") + + httpResp, err := c.httpClient.Do(httpReq) + if err != nil { + log.Warn().Err(err).Msg("failed to fetch required checks status") + return nil + } + defer httpResp.Body.Close() + + var gqlResp graphqlResponse + if err := json.NewDecoder(httpResp.Body).Decode(&gqlResp); err != nil { + log.Warn().Err(err).Msg("failed to decode required checks response") + return nil + } + if len(gqlResp.Errors) > 0 { + log.Warn().Str("error", gqlResp.Errors[0].Message).Msg("graphql error fetching required checks") + return nil + } + + // Build the set of required check names from config. + requiredSet := make(map[string]bool, len(c.config.Repo.RequiredChecks)) + for _, name := range c.config.Repo.RequiredChecks { + requiredSet[name] = true + } + + result := make(map[int]github.CheckStatus) + for _, pr := range pullRequests { + alias := fmt.Sprintf("pr_%d", pr.Number) + raw, ok := gqlResp.Data[alias] + if !ok { + continue + } + var prResult checkContextsResult + if err := json.Unmarshal(raw, &prResult); err != nil { + log.Warn().Err(err).Int("pr", pr.Number).Msg("failed to unmarshal check contexts for PR") + continue + } + if len(prResult.Commits.Nodes) == 0 { + continue + } + commit := prResult.Commits.Nodes[0].Commit + if commit.StatusCheckRollup == nil { + // No checks configured — treat as pass + result[pr.Number] = github.CheckStatusPass + continue + } + result[pr.Number] = computeRequiredCheckStatus(commit.StatusCheckRollup.Contexts.Nodes, requiredSet) + } + + return result +} + +// contextName returns the display name for a check context node. +// For CheckRun nodes this is the Name field; for StatusContext nodes it's the Context field. +func contextName(ctx checkContextNode) string { + if ctx.TypeName == "StatusContext" { + return ctx.Context + } + return ctx.Name +} + +// computeRequiredCheckStatus determines the aggregate check status considering +// only the checks whose name/context appears in requiredChecks. +// If a required check hasn't reported yet (not present in contexts), it is +// treated as pending. +func computeRequiredCheckStatus(contexts []checkContextNode, requiredChecks map[string]bool) github.CheckStatus { + // Track which required checks we've seen + seen := make(map[string]bool, len(requiredChecks)) + hasPending := false + hasFail := false + + for _, ctx := range contexts { + name := contextName(ctx) + if !requiredChecks[name] { + continue + } + seen[name] = true + + switch ctx.TypeName { + case "CheckRun": + switch ctx.Status { + case "COMPLETED": + if ctx.Conclusion == nil { + hasFail = true + } else { + switch *ctx.Conclusion { + case "SUCCESS", "NEUTRAL", "SKIPPED": + // pass + default: + hasFail = true + } + } + default: + // IN_PROGRESS, QUEUED, REQUESTED, WAITING, PENDING + hasPending = true + } + case "StatusContext": + switch ctx.State { + case "SUCCESS": + // pass + case "PENDING", "EXPECTED": + hasPending = true + default: + hasFail = true + } + } + } + + // Any required check that hasn't reported yet is pending + for name := range requiredChecks { + if !seen[name] { + hasPending = true + } + } + + if hasFail { + return github.CheckStatusFail + } + if hasPending { + return github.CheckStatusPending + } + return github.CheckStatusPass +} + func check(err error) { if err != nil { msg := err.Error() diff --git a/github/githubclient/client_test.go b/github/githubclient/client_test.go index c371fcf0..9df27e25 100644 --- a/github/githubclient/client_test.go +++ b/github/githubclient/client_test.go @@ -528,3 +528,228 @@ func TestMatchPullRequestStack(t *testing.T) { }) } } + +func TestComputeRequiredCheckStatus(t *testing.T) { + strPtr := func(s string) *string { return &s } + + tests := []struct { + name string + contexts []checkContextNode + requiredChecks map[string]bool + expect github.CheckStatus + }{ + // === Basic cases === + { + name: "NoContexts_NoRequired", + contexts: []checkContextNode{}, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusPending, // required check hasn't reported yet + }, + { + name: "NoContexts_EmptyRequired", + contexts: []checkContextNode{}, + requiredChecks: map[string]bool{}, + expect: github.CheckStatusPass, + }, + + // === CheckRun states === + { + name: "CheckRun_RequiredPasses", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci", Status: "COMPLETED", Conclusion: strPtr("SUCCESS")}, + }, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusPass, + }, + { + name: "CheckRun_RequiredFails", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci", Status: "COMPLETED", Conclusion: strPtr("FAILURE")}, + }, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusFail, + }, + { + name: "CheckRun_RequiredPending", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci", Status: "IN_PROGRESS"}, + }, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusPending, + }, + { + name: "CheckRun_RequiredQueued", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci", Status: "QUEUED"}, + }, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusPending, + }, + { + name: "CheckRun_NeutralPasses", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci", Status: "COMPLETED", Conclusion: strPtr("NEUTRAL")}, + }, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusPass, + }, + { + name: "CheckRun_SkippedPasses", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci", Status: "COMPLETED", Conclusion: strPtr("SKIPPED")}, + }, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusPass, + }, + { + name: "CheckRun_TimedOut", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci", Status: "COMPLETED", Conclusion: strPtr("TIMED_OUT")}, + }, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusFail, + }, + { + name: "CheckRun_Cancelled", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci", Status: "COMPLETED", Conclusion: strPtr("CANCELLED")}, + }, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusFail, + }, + { + name: "CheckRun_NilConclusion", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci", Status: "COMPLETED", Conclusion: nil}, + }, + requiredChecks: map[string]bool{"ci": true}, + expect: github.CheckStatusFail, + }, + + // === StatusContext states === + { + name: "StatusContext_RequiredSuccess", + contexts: []checkContextNode{ + {TypeName: "StatusContext", Context: "ci/build", State: "SUCCESS"}, + }, + requiredChecks: map[string]bool{"ci/build": true}, + expect: github.CheckStatusPass, + }, + { + name: "StatusContext_RequiredFailure", + contexts: []checkContextNode{ + {TypeName: "StatusContext", Context: "ci/build", State: "FAILURE"}, + }, + requiredChecks: map[string]bool{"ci/build": true}, + expect: github.CheckStatusFail, + }, + { + name: "StatusContext_RequiredPending", + contexts: []checkContextNode{ + {TypeName: "StatusContext", Context: "ci/build", State: "PENDING"}, + }, + requiredChecks: map[string]bool{"ci/build": true}, + expect: github.CheckStatusPending, + }, + { + name: "StatusContext_RequiredExpected", + contexts: []checkContextNode{ + {TypeName: "StatusContext", Context: "ci/build", State: "EXPECTED"}, + }, + requiredChecks: map[string]bool{"ci/build": true}, + expect: github.CheckStatusPending, + }, + + // === Filtering: non-required checks are ignored === + { + name: "NonRequiredFails_RequiredPasses", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "required-ci", Status: "COMPLETED", Conclusion: strPtr("SUCCESS")}, + {TypeName: "CheckRun", Name: "optional-lint", Status: "COMPLETED", Conclusion: strPtr("FAILURE")}, + }, + requiredChecks: map[string]bool{"required-ci": true}, + expect: github.CheckStatusPass, + }, + { + name: "MixedTypes_NonRequiredFails_RequiredPasses", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "required-ci", Status: "COMPLETED", Conclusion: strPtr("SUCCESS")}, + {TypeName: "StatusContext", Context: "ci/build", State: "SUCCESS"}, + {TypeName: "CheckRun", Name: "optional-lint", Status: "COMPLETED", Conclusion: strPtr("FAILURE")}, + {TypeName: "StatusContext", Context: "optional/coverage", State: "FAILURE"}, + }, + requiredChecks: map[string]bool{"required-ci": true, "ci/build": true}, + expect: github.CheckStatusPass, + }, + { + name: "MixedTypes_RequiredFails", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "required-ci", Status: "COMPLETED", Conclusion: strPtr("SUCCESS")}, + {TypeName: "StatusContext", Context: "ci/build", State: "FAILURE"}, + }, + requiredChecks: map[string]bool{"required-ci": true, "ci/build": true}, + expect: github.CheckStatusFail, + }, + + // === Precedence === + { + name: "FailTakesPrecedenceOverPending", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci-1", Status: "COMPLETED", Conclusion: strPtr("FAILURE")}, + {TypeName: "CheckRun", Name: "ci-2", Status: "IN_PROGRESS"}, + }, + requiredChecks: map[string]bool{"ci-1": true, "ci-2": true}, + expect: github.CheckStatusFail, + }, + { + name: "AllRequiredPass_MultipleChecks", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci-1", Status: "COMPLETED", Conclusion: strPtr("SUCCESS")}, + {TypeName: "CheckRun", Name: "ci-2", Status: "COMPLETED", Conclusion: strPtr("SUCCESS")}, + {TypeName: "CheckRun", Name: "ci-3", Status: "COMPLETED", Conclusion: strPtr("SUCCESS")}, + }, + requiredChecks: map[string]bool{"ci-1": true, "ci-2": true, "ci-3": true}, + expect: github.CheckStatusPass, + }, + + // === Missing required check (not yet reported) === + { + name: "RequiredCheckNotReportedYet", + contexts: []checkContextNode{ + {TypeName: "CheckRun", Name: "ci-1", Status: "COMPLETED", Conclusion: strPtr("SUCCESS")}, + }, + requiredChecks: map[string]bool{"ci-1": true, "ci-2": true}, + expect: github.CheckStatusPending, + }, + + // === Real-world scenario: Semaphore + GitHub Actions === + { + name: "RealWorld_SemaphoreFails_OnlyItRequired", + contexts: []checkContextNode{ + {TypeName: "StatusContext", Context: "ci/semaphoreci/pr: test", State: "FAILURE"}, + {TypeName: "CheckRun", Name: "PR Policy Review", Status: "COMPLETED", Conclusion: strPtr("NEUTRAL")}, + {TypeName: "CheckRun", Name: "review", Status: "COMPLETED", Conclusion: strPtr("CANCELLED")}, + {TypeName: "CheckRun", Name: "repl-controller", Status: "COMPLETED", Conclusion: strPtr("SUCCESS")}, + }, + requiredChecks: map[string]bool{"ci/semaphoreci/pr: test": true}, + expect: github.CheckStatusFail, + }, + { + name: "RealWorld_SemaphorePasses_OthersFail", + contexts: []checkContextNode{ + {TypeName: "StatusContext", Context: "ci/semaphoreci/pr: test", State: "SUCCESS"}, + {TypeName: "CheckRun", Name: "review", Status: "COMPLETED", Conclusion: strPtr("CANCELLED")}, + {TypeName: "CheckRun", Name: "optional", Status: "COMPLETED", Conclusion: strPtr("FAILURE")}, + }, + requiredChecks: map[string]bool{"ci/semaphoreci/pr: test": true}, + expect: github.CheckStatusPass, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + actual := computeRequiredCheckStatus(tc.contexts, tc.requiredChecks) + require.Equal(t, tc.expect, actual) + }) + } +} diff --git a/github/pullrequest.go b/github/pullrequest.go index fe473df2..dd1f8bbb 100644 --- a/github/pullrequest.go +++ b/github/pullrequest.go @@ -26,11 +26,12 @@ type PullRequest struct { LocalCommitHash string } -type checkStatus int +// CheckStatus represents the aggregate status of GitHub checks on a pull request +type CheckStatus int const ( // CheckStatusUnknown - CheckStatusUnknown checkStatus = iota + CheckStatusUnknown CheckStatus = iota // CheckStatusPending when checks are still running CheckStatusPending @@ -45,7 +46,7 @@ const ( // PullRequestMergeStatus is the merge status of a pull request type PullRequestMergeStatus struct { // ChecksPass is the status of GitHub checks - ChecksPass checkStatus + ChecksPass CheckStatus // ReviewApproved is true when a pull request is approved by a fellow reviewer ReviewApproved bool @@ -237,7 +238,14 @@ func (pr *PullRequest) String(config *config.Config) string { return line } -func (cs checkStatus) String(config *config.Config) string { +// TextString returns a plain text representation of the pull request: " : " +func (pr *PullRequest) TextString(config *config.Config) string { + prURL := fmt.Sprintf("https://%s/%s/%s/pull/%d", + config.Repo.GitHubHost, config.Repo.GitHubRepoOwner, config.Repo.GitHubRepoName, pr.Number) + return fmt.Sprintf("%s : %s", prURL, pr.Title) +} + +func (cs CheckStatus) String(config *config.Config) string { icons := statusBitIcons(config) if config.Repo.RequireChecks { switch cs { diff --git a/github/pullrequest_test.go b/github/pullrequest_test.go index 8e1c5744..998fd1bf 100644 --- a/github/pullrequest_test.go +++ b/github/pullrequest_test.go @@ -25,7 +25,7 @@ func TestMergable(t *testing.T) { } } - pr := func(checks checkStatus, approved bool, noConflics bool, stacked bool) *PullRequest { + pr := func(checks CheckStatus, approved bool, noConflics bool, stacked bool) *PullRequest { return &PullRequest{ MergeStatus: PullRequestMergeStatus{ ChecksPass: checks, @@ -69,7 +69,7 @@ func TestReady(t *testing.T) { } } - pr := func(checks checkStatus, wip bool, approved bool, noConflics bool, stacked bool) *PullRequest { + pr := func(checks CheckStatus, wip bool, approved bool, noConflics bool, stacked bool) *PullRequest { return &PullRequest{ MergeStatus: PullRequestMergeStatus{ ChecksPass: checks, @@ -117,7 +117,7 @@ func TestStatusString(t *testing.T) { } } - pr := func(checks checkStatus, approved bool, noConflics bool, stacked bool) *PullRequest { + pr := func(checks CheckStatus, approved bool, noConflics bool, stacked bool) *PullRequest { return &PullRequest{ MergeStatus: PullRequestMergeStatus{ ChecksPass: checks, @@ -241,3 +241,26 @@ func TestString(t *testing.T) { assert.Equal(t, test.expect, test.pr.String(test.cfg), fmt.Sprintf("case %d failed", i)) } } + +func TestTextString(t *testing.T) { + cfg := &config.Config{ + Repo: &config.RepoConfig{ + GitHubHost: "github.com", + GitHubRepoOwner: "testowner", + GitHubRepoName: "testrepo", + }, + User: &config.UserConfig{}, + } + + pr := &PullRequest{ + Number: 42, + Title: "Add new feature", + } + assert.Equal(t, "https://github.com/testowner/testrepo/pull/42 : Add new feature", pr.TextString(cfg)) + + pr2 := &PullRequest{ + Number: 7, + Title: "Fix bug in parser", + } + assert.Equal(t, "https://github.com/testowner/testrepo/pull/7 : Fix bug in parser", pr2.TextString(cfg)) +} diff --git a/readme.md b/readme.md index 05f791a9..30fcee29 100644 --- a/readme.md +++ b/readme.md @@ -1,22 +1,45 @@ ![logo](docs/git_spr_logo.png) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) [![Build](https://github.com/ejoffe/spr/actions/workflows/ci.yml/badge.svg)](https://github.com/ejoffe/spr/actions/workflows/ci.yml) -[![ReportCard](https://goreportcard.com/badge/github.com/ejoffe/spr)](https://goreportcard.com/report/github.com/ejoffe/spr) -[![Doc](https://godoc.org/github.com/ejoffe/spr?status.svg)](https://godoc.org/github.com/ejoffe/spr) -[![Release](https://img.shields.io/github/release/ejoffe/spr.svg)](https://GitHub.com/ejoffe/spr/releases/) -[![Join the chat at https://gitter.im/ejoffe-spr/community](https://badges.gitter.im/ejoffe-spr/community.svg)](https://gitter.im/ejoffe-spr/community?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) +[![Release](https://img.shields.io/github/release/ejoffe/spr.svg)](https://GitHub.com/ejoffe/spr/releases/) + +**Each commit becomes a pull request. Stop juggling branches.** + +`git spr` manages stacked pull requests on GitHub so you don't have to. Write commits on a single branch, and spr turns each one into its own pull request -- kept in sync, correctly ordered, and ready to merge. ![terminal cast](docs/git_spr_cast.gif) -# Stacked Pull Requests on GitHub +## Why stacked PRs? + +- **Small PRs get reviewed faster.** A 50-line change gets meaningful feedback; a 500-line change gets "looks good." +- **No more branch gymnastics.** Stop creating `feature-part-1`, `feature-part-2`, rebasing one onto the other, and resolving conflicts between them. +- **Ship incrementally.** Land the database migration today, the API tomorrow, the UI the day after -- each reviewed and merged independently. +- **Works with native GitHub.** No extra services, no custom merge bots. Just pull requests and branches, managed for you. + +## Quick Start + +Install via brew, nix, or [download a binary](https://github.com/ejoffe/spr/releases): + +```shell +brew install ejoffe/tap/spr # macOS/Linux +nix profile install github:ejoffe/spr # Nix +``` + +Then use it like normal git -- just replace `git push` + manual PR creation with `git spr update`: -Easily manage stacks of pull requests on GitHub. -`git spr` is a client side tool that achieves a simple streamlined stacked diff workflow using github pull requests and branches. `git spr` manages your pull request stacks for you, so you don't have to. +```shell +git commit -m "Add user authentication" +git commit -m "Add login page" +git commit -m "Add session management" + +git spr update # creates 3 pull requests, stacked in order +git spr status # show status of your stack +git spr merge # merge everything that's ready +``` -With `git spr` each commit becomes a pull request, and each branch becomes a stack of pull requests. This allows for multiple commits to be stacked on top of each other in a single branch, avoiding the overhead of starting a new branch for every new change or feature. Small changes and pull requests are easy and fast to achieve. One doesn't have to worry about stacked branches on top of each other and managing complicated pull request stacks. The end result is a more streamlined faster software development cycle. +That's it. Each commit is a PR. Amend a commit and run `git spr update` again to sync changes. -Commands --------- +## Commands | Command | Aliases | Description | |---------|---------|-------------| @@ -31,8 +54,7 @@ Commands **Global flags:** `--detail` (show status bit headers), `--verbose` (log git commands and GitHub API calls), `--debug`, `--profile` -Installation ------------- +## Installation ### Brew ```shell @@ -40,13 +62,6 @@ brew tap ejoffe/homebrew-tap brew install ejoffe/tap/spr ``` -### Apt -```shell -echo "deb [trusted=yes] https://apt.fury.io/inigolabs/ /" | sudo tee /etc/apt/sources.list.d/inigolabs.list -sudo apt update -sudo apt install spr -``` - ### Nix ```shell nix profile install github:ejoffe/spr @@ -56,37 +71,45 @@ Or run without installing: nix run github:ejoffe/spr ``` -### Manual -Download the pre-compiled binaries from the [releases page](https://github.com/ejoffe/spr/releases) and copy to your bin path. +### Apt +```shell +echo "deb [trusted=yes] https://apt.fury.io/inigolabs/ /" | sudo tee /etc/apt/sources.list.d/inigolabs.list +sudo apt update +sudo apt install spr +``` + +### Binary +Download pre-compiled binaries from the [releases page](https://github.com/ejoffe/spr/releases). ### From source -Install [goreleaser](https://goreleaser.com/) and run make. Binaries can be found in the **dist** directory. ```shell -make bin +make bin # requires goreleaser; binaries output to dist/ ``` -Workflow --------- -Commit your changes to a branch as you normally do. Note that every commit will end up becoming a pull request. +## Usage Guide + +### Workflow + +Commit your changes to a branch as you normally do. Every commit becomes a pull request. + ```shell -> touch feature_1 -> git add feature_1 -> git commit -m "Feature 1" -> touch feature_2 -> git add feature_2 -> git commit -m "Feature 2" -> touch feature_3 -> git add feature_3 -> git commit -m "Feature 3" +git add feature_1.go +git commit -m "Feature 1" +git add feature_2.go +git commit -m "Feature 2" +git add feature_3.go +git commit -m "Feature 3" + +git spr update ``` -The subject of the commit message will be the title of the pull request, and the body of the message will be the body of the pull request. -If you have a work in progress change that you want to commit, but don't want to create a pull request yet, start the commit message with all caps **WIP**. The spr script will not create a pull request for any commit which starts with WIP, when you are ready to create a pull request remove the WIP. -There is no need to create new branches for every change, and you don't have to call git push to get your code to github. Instead just call `git spr update`. +The commit subject becomes the PR title; the commit body becomes the PR description. There's no need to create branches or call `git push` -- `git spr update` handles everything. -Managing Pull Requests ----------------------- -Run `git spr update` to sync your whole commit stack to github and create pull requests for each new commit in the stack. If a commit was amended the pull request will be updated automatically. The command outputs a list of your open pull requests and their status. `git spr update` pushes your commits to github and creates pull requests for you, so you don't need to call git push or open pull requests manually in the UI. +**Work in progress:** Prefix a commit message with **WIP** to skip PR creation for that commit. Remove the prefix when you're ready. + +### Updating pull requests + +Run `git spr update` to sync your entire stack. New commits get new PRs; amended commits update existing PRs automatically. ```shell > git spr update @@ -101,13 +124,12 @@ Run `git spr update` to sync your whole commit stack to github and create pull r | `--reviewer` | `-r` | Add reviewers to newly created pull requests | | `--no-rebase` | `--nr` | Disable rebasing (also supports `SPR_NOREBASE` env var) | -Amending Commits ----------------- -When you need to update a commit, either to fix tests, update code based on review comments, or just need to change something because you feel like it. You should amend the commit. -Use `git amend` to easily amend your changes anywhere in the stack. Stage the files you want to amend, and instead of calling git commit, use `git amend` and choose the commit you want to amend when prompted. +### Amending commits + +Stage your changes, then use `git spr amend` to pick which commit to amend: + ```shell -> touch feature_2 -> git add feature_2 +> git add feature_2.go > git spr amend 3 : 5cba235d : Feature 3 2 : 4dc2c5b2 : Feature 2 @@ -117,9 +139,9 @@ Commit to amend [1-3]: 2 Use `--update` (`-u`) to automatically run `git spr update` after amending. -Editing Commits ---------------- -Use `git spr edit` to interactively edit a commit in the stack. This starts an interactive rebase session where you can make changes to the selected commit. +### Editing commits + +Use `git spr edit` to start an interactive rebase session on a specific commit: ```shell > git spr edit @@ -129,41 +151,15 @@ Use `git spr edit` to interactively edit a commit in the stack. This starts an i Commit to edit [1-3]: 2 ``` -Once you've made your changes, finish the session with `git spr edit --done`. Use `--update` (`-u`) with `--done` to also run `git spr update` afterwards. If you need to cancel, use `git spr edit --abort`. - -Syncing Your Stack ------------------- -Use `git spr sync` to synchronize your local stack with the remote. This is useful when pull requests have been updated on GitHub (e.g., after a merge) and you need to bring your local commits in line with the remote state. - -Merge Status Bits ------------------ -Each pull request has four merge status bits signifying the request's ability to be merged. For a request to be merged, all required status bits need to show ✅. - -| Bit | ⌛ | ❌ | ✅ | ➖ | -|-----|---|---|---|---| -| 1. Checks | pending | failed | pass | not required | -| 2. Approval | — | not approved | approved | not required | -| 3. Conflicts | — | has conflicts | no conflicts | — | -| 4. Stack | — | blocked below | all clear | — | +Finish with `git spr edit --done` (add `-u` to also update). Cancel with `git spr edit --abort`. -Checks and approval requirements can be configured via `requireChecks` and `requireApproval` in `.spr.yml`. +### Syncing -Show Current Pull Requests --------------------------- -Use `git spr status` to see the status of your pull request stack. In the following case three pull requests are all green and ready to be merged, and one pull request is waiting for review approval. +Use `git spr sync` to pull remote changes into your local stack. Useful after PRs have been merged or updated on GitHub. -```shell -> git spr status -[✅❌✅✅] 61: Feature 4 -[✅✅✅✅] 60: Feature 3 -[✅✅✅✅] 59: Feature 2 -[✅✅✅✅] 58: Feature 1 -``` +### Merging -Merging Pull Requests ---------------------- -Your pull requests are stacked. Don't use the GitHub UI to merge pull requests, if you do it in the wrong order, you'll end up pushing one pull request into another, which is probably not what you want. Instead just use `git spr merge` and you can merge all the pull requests that are mergeable in one shot. Status for the remaining pull requests will be printed after the merged requests. -In order to merge all pull requests in one shot without causing extra github checks to trigger, spr finds the top mergeable pull request. It then combines all the commits up to this pull request into one single pull request, merges this request, and closes the rest of the pull requests. This is a bit surprising at first, and has some side effects, but no better solution has been found to date. +Use `git spr merge` instead of the GitHub UI to merge in the correct order: ```shell > git spr merge @@ -173,74 +169,113 @@ MERGED #60 Feature 3 [✅❌✅✅] 61: Feature 4 ``` -To merge only part of the stack use the `--count` flag with the number of pull requests in the stack that you would like to merge. Pull requests will be merged from the bottom of the stack upwards. +spr finds the top mergeable PR in the stack, combines all commits up to it into a single PR, merges it, and closes the intermediate PRs. This avoids triggering redundant CI runs. -```shell -> git spr merge --count 2 -MERGED #58 Feature 1 -MERGED #59 Feature 2 +Use `--count N` to merge only the bottom N pull requests. + +### Merge status bits + +Each PR shows four status bits: + +``` [✅❌✅✅] 61: Feature 4 -[✅✅✅✅] 60: Feature 3 -``` - -By default merges are done using the rebase merge method, this can be changed using the `mergeMethod` configuration. - -Running Pre-Merge Checks -------------------------- -Use `git spr check` to run a pre-merge check command configured via the `mergeCheck` repository setting. This lets you enforce that tests or linters pass before merging. - -Starting a New Stack ---------------------- -Starting a new stack works by creating a new branch. For example, if you want to start a new stack from the latest pushed state of your current branch, use `git checkout -b new_branch @{push}`. - -Configuration -------------- -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. - -| 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) | -| githubRemote | str | origin | github remote name to use | -| githubBranch | str | main | 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 | -| prTemplateType | str | stack | PR template type, valid values: [stack, basic, why_what, custom]. If prTemplatePath is provided, this is automatically set to "custom" | -| prTemplatePath | str | | path to PR template file (e.g. .github/PULL_REQUEST_TEMPLATE/pull_request_template.md). When provided, prTemplateType is automatically set to "custom" | -| prTemplateInsertStart | str | | text marker in PR template that determines where to insert commit body (used with custom template type) | -| prTemplateInsertEnd | str | | text marker in PR template that determines where to end commit body insertion (used with custom template type) | -| mergeCheck | str | | enforce a pre-merge check using 'git spr check' | -| forceFetchTags | bool | false | also fetch tags when running 'git spr update' | -| showPrTitlesInStack | bool | false | show PR titles in stack description within pull request body | -| branchPushIndividually | bool | false | push branches individually instead of atomically (only enable to avoid timeouts) | -| defaultReviewers | list | | default reviewers to add to each pull request | - -| User Config | Type | Default | Description | -| -------------------- | ---- | ------- | --------------------------------------------------------------- | -| showPRLink | bool | true | show full pull request http link | -| logGitCommands | bool | false | log git commands to stdout (enabled by `--verbose`) | -| logGitHubCalls | bool | false | log GitHub API calls to stdout (enabled by `--verbose`) | -| 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 | -| deleteMergedBranches | bool | false | delete branches after prs are merged | -| shortPRLink | bool | false | show pull request links as clickable PR-<number> instead of full URL | -| showCommitID | bool | false | show first 8 characters of commit hash for each pull request | - -Happy Coding! -------------- -If you find a bug, feel free to open an issue. Pull requests are welcome. - -If you find this tool as useful as I do, add a **star** and tell your fellow GitHubbers. - -License -------- - -- [MIT License](LICENSE) + │ │ │ └─ stack: all PRs below are ready + │ │ └──── conflicts: no merge conflicts + │ └─────── approval: PR is approved + └────────── checks: CI checks pass +``` + +| Bit | ⌛ | ❌ | ✅ | ➖ | +|-----|---|---|---|---| +| Checks | pending | failed | pass | not required | +| Approval | -- | not approved | approved | not required | +| Conflicts| -- | has conflicts| no conflicts| -- | +| Stack | -- | blocked below| all clear | -- | + +Configure check and approval requirements with `requireChecks`, `requiredChecks`, and `requireApproval` in `.spr.yml`. When `requiredChecks` lists specific check names, only those checks are evaluated -- all others are ignored. This is useful when optional checks (e.g. linters, deploy previews) would otherwise cause the status to show as failed. + +### Starting a new stack + +Create a new branch from the latest pushed state: + +```shell +git checkout -b new_stack @{push} +``` + +## Configuration + +Configuration is created automatically on first run. Repository config lives in `.spr.yml` at the repo root; user config lives in `~/.spr.yml`. + +<details> +<summary><strong>Repository configuration</strong> (.spr.yml)</summary> + +| Setting | Type | Default | Description | +|---------|------|---------|-------------| +| `requireChecks` | bool | `true` | Require checks to pass in order to merge | +| `requiredChecks` | list | | List of check names that must pass. When set, only these checks are evaluated; all others are ignored | +| `requireApproval` | bool | `true` | Require PR approval in order to merge | +| `githubRepoOwner` | str | | GitHub owner (auto-detected from git remote) | +| `githubRepoName` | str | | GitHub repository name (auto-detected from git remote) | +| `githubRemote` | str | `origin` | Git remote name to use | +| `githubBranch` | str | `main` | Target branch for pull requests | +| `githubHost` | str | `github.com` | GitHub host (update for GitHub Enterprise) | +| `mergeMethod` | str | `rebase` | Merge method: `rebase`, `squash`, or `merge` | +| `mergeQueue` | bool | `false` | Use GitHub merge queue | +| `prTemplateType` | str | `stack` | PR template: `stack`, `basic`, `why_what`, or `custom` | +| `prTemplatePath` | str | | Path to custom PR template file (auto-sets type to `custom`) | +| `prTemplateInsertStart` | str | | Marker in custom template for commit body insertion start | +| `prTemplateInsertEnd` | str | | Marker in custom template for commit body insertion end | +| `mergeCheck` | str | | Command to run with `git spr check` before merging | +| `forceFetchTags` | bool | `false` | Fetch tags during `git spr update` | +| `showPrTitlesInStack` | bool | `false` | Show PR titles in stack description within PR body | +| `branchPushIndividually` | bool | `false` | Push branches one at a time instead of atomically | +| `defaultReviewers` | list | | Reviewers to add to every new pull request | + +Example `.spr.yml`: + +```yaml +requireChecks: true +requiredChecks: + - "ci/test" + - "ci/build" +requireApproval: true +mergeMethod: squash +defaultReviewers: + - teammate +``` + +</details> + +<details> +<summary><strong>User configuration</strong> (~/.spr.yml)</summary> + +| Setting | Type | Default | Description | +|---------|------|---------|-------------| +| `showPRLink` | bool | `true` | Show full pull request URL | +| `shortPRLink` | bool | `false` | Show clickable `PR-<number>` instead of full URL | +| `showCommitID` | bool | `false` | Show first 8 characters of commit hash | +| `logGitCommands` | bool | `false` | Log git commands to stdout | +| `logGitHubCalls` | bool | `false` | Log GitHub API calls to stdout | +| `statusBitsHeader` | bool | `true` | Show status bit type headers | +| `statusBitsEmojis` | bool | `true` | Use emoji status bits | +| `createDraftPRs` | bool | `false` | Create new PRs as drafts | +| `preserveTitleAndBody` | bool | `false` | Don't overwrite PR title and body on update | +| `noRebase` | bool | `false` | Skip rebasing on `git spr update` | +| `deleteMergedBranches` | bool | `false` | Delete branches after PRs are merged | +| `branchPrefix` | str | `spr` | Prefix for spr-managed branch names | + +</details> + +## How it compares + +spr is similar to [Graphite](https://graphite.dev), [ghstack](https://github.com/ezyang/ghstack), and [Gerrit](https://www.gerritcodereview.com/)'s stacked review model -- but works purely with GitHub's native pull requests. No extra service, no custom merge bot, no lock-in. + +## Contributing + +Found a bug? [Open an issue.](https://github.com/ejoffe/spr/issues) Pull requests are welcome. + +If you find spr useful, a star helps others discover it. + +## License + +[MIT License](LICENSE) diff --git a/spr/spr.go b/spr/spr.go index 4ee50bea..f848800c 100644 --- a/spr/spr.go +++ b/spr/spr.go @@ -42,6 +42,7 @@ type stackediff struct { gitcmd git.GitInterface profiletimer profiletimer.Timer DetailEnabled bool + TextEnabled bool output io.Writer input io.Reader @@ -171,8 +172,9 @@ func (sd *stackediff) EditCommitDone(ctx context.Context, update bool) { return } - // Stage all changes - sd.gitcmd.MustGit("add -A", nil) + // Stage modifications and deletions to tracked files only. + // Using -u instead of -A avoids accidentally staging untracked files. + sd.gitcmd.MustGit("add -u", nil) // Check if we're resolving a rebase conflict or at the initial edit stop. // Git creates .git/REBASE_HEAD when a rebase stops due to a conflict, @@ -517,7 +519,12 @@ func (sd *stackediff) StatusPullRequests(ctx context.Context) { sd.profiletimer.Step("StatusPullRequests::Start") githubInfo := sd.github.GetInfo(ctx, sd.gitcmd) - if len(githubInfo.PullRequests) == 0 { + if sd.TextEnabled { + for i := len(githubInfo.PullRequests) - 1; i >= 0; i-- { + pr := githubInfo.PullRequests[i] + fmt.Fprintf(sd.output, "%s\n", pr.TextString(sd.config)) + } + } else if len(githubInfo.PullRequests) == 0 { fmt.Fprintf(sd.output, "pull request stack is empty\n") } else { if sd.DetailEnabled { diff --git a/spr/spr_test.go b/spr/spr_test.go index 0379ee70..74a484ee 100644 --- a/spr/spr_test.go +++ b/spr/spr_test.go @@ -1257,3 +1257,43 @@ func TestEditCommitAbortNotEditing(t *testing.T) { s.EditCommitAbort(ctx) require.Equal(t, "No edit session in progress.\n", output.String()) } + +func TestStatusPullRequestsTextMode(t *testing.T) { + s, _, githubmock, _, output := makeTestObjects(t, true) + assert := require.New(t) + ctx := context.Background() + s.TextEnabled = true + + s.config.Repo.GitHubHost = "github.com" + s.config.Repo.GitHubRepoOwner = "testowner" + s.config.Repo.GitHubRepoName = "testrepo" + + // Text mode with empty stack should print nothing + githubmock.ExpectGetInfo() + s.StatusPullRequests(ctx) + assert.Equal("", output.String()) + githubmock.ExpectationsMet() + output.Reset() + + // Text mode with PRs should print "<url> : <title>" for each PR + githubmock.Info.PullRequests = []*github.PullRequest{ + { + Number: 1, + Title: "first PR", + Commit: git.Commit{CommitID: "00000001"}, + }, + { + Number: 2, + Title: "second PR", + Commit: git.Commit{CommitID: "00000002"}, + }, + } + githubmock.ExpectGetInfo() + s.StatusPullRequests(ctx) + lines := strings.Split(strings.TrimRight(output.String(), "\n"), "\n") + assert.Equal(2, len(lines)) + // PRs are printed in reverse order (top of stack first) + assert.Equal("https://github.com/testowner/testrepo/pull/2 : second PR", lines[0]) + assert.Equal("https://github.com/testowner/testrepo/pull/1 : first PR", lines[1]) + githubmock.ExpectationsMet() +}