diff --git a/README.md b/README.md index d1eb364..af1f849 100644 --- a/README.md +++ b/README.md @@ -454,18 +454,6 @@ gh stack unstack gh stack unstack --local ``` -### `gh stack merge` - -Merge a stack of PRs. - -``` -gh stack merge -``` - -Merges the specified PR and all PRs below it in the stack. - -> **Note:** This command is not yet implemented. Running it prints a notice. - ### Navigation Move between branches in the current stack without having to remember branch names. diff --git a/cmd/merge.go b/cmd/merge.go deleted file mode 100644 index dc314ce..0000000 --- a/cmd/merge.go +++ /dev/null @@ -1,113 +0,0 @@ -package cmd - -import ( - "fmt" - - "github.com/cli/go-gh/v2/pkg/browser" - "github.com/cli/go-gh/v2/pkg/prompter" - "github.com/github/gh-stack/internal/config" - "github.com/github/gh-stack/internal/stack" - "github.com/spf13/cobra" -) - -func MergeCmd(cfg *config.Config) *cobra.Command { - cmd := &cobra.Command{ - Use: "merge []", - Short: "Merge a stack of PRs", - Long: `Merges the specified PR and all PRs below it in the stack. - -Accepts a PR URL, PR number, or branch name. When run without -arguments, operates on the current branch's PR.`, - Args: cobra.MaximumNArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - var target string - if len(args) > 0 { - target = args[0] - } - return runMerge(cfg, target) - }, - } - - return cmd -} - -func runMerge(cfg *config.Config, target string) error { - // Standard stack loading and validation. - result, err := loadStack(cfg, "") - if err != nil { - return ErrNotInStack - } - s := result.Stack - currentBranch := result.CurrentBranch - - // Sync PR state from GitHub so merge status is up to date. - _ = syncStackPRs(cfg, s) - - // Persist the refreshed PR state. - stack.SaveNonBlocking(result.GitDir, result.StackFile) - - // Resolve which branch to operate on. - var br *stack.BranchRef - if target != "" { - _, br, err = resolvePR(cfg, result.StackFile, target) - if err != nil { - cfg.Errorf("%s", err) - return ErrNotInStack - } - } else { - idx := s.IndexOf(currentBranch) - if idx < 0 { - if s.IsFullyMerged() { - cfg.Successf("All PRs in this stack have already been merged") - return nil - } - cfg.Errorf("current branch %q is not a stack branch (it may be the trunk)", currentBranch) - return ErrNotInStack - } - br = &s.Branches[idx] - } - - if br.PullRequest == nil { - cfg.Errorf("no pull request found for branch %q", br.Branch) - cfg.Printf(" Run %s to create PRs for this stack.", cfg.ColorCyan("gh stack submit")) - return ErrSilent - } - - if br.IsMerged() { - cfg.Successf("PR %s has already been merged", cfg.PRLink(br.PullRequest.Number, br.PullRequest.URL)) - cfg.Printf(" %s", br.PullRequest.URL) - return nil - } - - prURL := br.PullRequest.URL - prLink := cfg.PRLink(br.PullRequest.Number, prURL) - - cfg.Warningf("Merging stacked PRs from the CLI is not yet supported") - - if cfg.IsInteractive() { - p := prompter.New(cfg.In, cfg.Out, cfg.Err) - openWeb, promptErr := p.Confirm( - fmt.Sprintf("Open %s in your browser?", prLink), true) - if promptErr != nil { - if isInterruptError(promptErr) { - printInterrupt(cfg) - return nil - } - cfg.Errorf("prompt failed: %s", promptErr) - return nil - } - - if openWeb { - b := browser.New("", cfg.Out, cfg.Err) - if err := b.Browse(prURL); err != nil { - cfg.Warningf("failed to open browser: %s", err) - } else { - cfg.Successf("Opened %s in your browser", prLink) - return nil - } - } - } - - cfg.Printf(" You can merge this PR at: %s", prURL) - return nil -} diff --git a/cmd/merge_test.go b/cmd/merge_test.go deleted file mode 100644 index 3a5e282..0000000 --- a/cmd/merge_test.go +++ /dev/null @@ -1,354 +0,0 @@ -package cmd - -import ( - "io" - "testing" - - "github.com/github/gh-stack/internal/config" - "github.com/github/gh-stack/internal/git" - "github.com/github/gh-stack/internal/github" - "github.com/github/gh-stack/internal/stack" - "github.com/stretchr/testify/assert" -) - -func newMergeMock(tmpDir, currentBranch string) *git.MockOps { - return &git.MockOps{ - GitDirFn: func() (string, error) { return tmpDir, nil }, - CurrentBranchFn: func() (string, error) { return currentBranch, nil }, - } -} - -func TestMerge_NoPullRequest(t *testing.T) { - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "feat-1"}, - }, - } - - tmpDir := t.TempDir() - writeStackFile(t, tmpDir, s) - - restore := git.SetOps(newMergeMock(tmpDir, "feat-1")) - defer restore() - - cfg, _, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{} - cmd := MergeCmd(cfg) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - err := cmd.Execute() - - cfg.Err.Close() - errOut, _ := io.ReadAll(errR) - output := string(errOut) - - assert.ErrorIs(t, err, ErrSilent) - assert.Contains(t, output, "no pull request found") - assert.Contains(t, output, "gh stack submit") -} - -func TestMerge_AlreadyMerged(t *testing.T) { - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "feat-1", PullRequest: &stack.PullRequestRef{ - Number: 42, - URL: "https://github.com/owner/repo/pull/42", - Merged: true, - }}, - }, - } - - tmpDir := t.TempDir() - writeStackFile(t, tmpDir, s) - - restore := git.SetOps(newMergeMock(tmpDir, "feat-1")) - defer restore() - - cfg, _, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{} - cmd := MergeCmd(cfg) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - err := cmd.Execute() - - cfg.Err.Close() - errOut, _ := io.ReadAll(errR) - output := string(errOut) - - assert.NoError(t, err) - assert.Contains(t, output, "already been merged") - assert.Contains(t, output, "https://github.com/owner/repo/pull/42") -} - -func TestMerge_FullyMergedStack(t *testing.T) { - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "feat-1", PullRequest: &stack.PullRequestRef{ - Number: 10, - URL: "https://github.com/owner/repo/pull/10", - Merged: true, - }}, - {Branch: "feat-2", PullRequest: &stack.PullRequestRef{ - Number: 11, - URL: "https://github.com/owner/repo/pull/11", - Merged: true, - }}, - }, - } - - tmpDir := t.TempDir() - writeStackFile(t, tmpDir, s) - - // On trunk with all PRs merged → fully merged message. - restore := git.SetOps(newMergeMock(tmpDir, "main")) - defer restore() - - cfg, _, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{} - cmd := MergeCmd(cfg) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - err := cmd.Execute() - - cfg.Err.Close() - errOut, _ := io.ReadAll(errR) - output := string(errOut) - - assert.NoError(t, err) - assert.Contains(t, output, "All PRs in this stack have already been merged") -} - -func TestMerge_OnTrunk(t *testing.T) { - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "feat-1", PullRequest: &stack.PullRequestRef{ - Number: 42, - URL: "https://github.com/owner/repo/pull/42", - }}, - }, - } - - tmpDir := t.TempDir() - writeStackFile(t, tmpDir, s) - - // Current branch is trunk, not a stack branch. - restore := git.SetOps(newMergeMock(tmpDir, "main")) - defer restore() - - cfg, _, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{} - cmd := MergeCmd(cfg) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - err := cmd.Execute() - - cfg.Err.Close() - errOut, _ := io.ReadAll(errR) - output := string(errOut) - - assert.ErrorIs(t, err, ErrNotInStack) - assert.Contains(t, output, "not a stack branch") -} - -func TestMerge_NonInteractive_PrintsURL(t *testing.T) { - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "feat-1", PullRequest: &stack.PullRequestRef{ - Number: 42, - URL: "https://github.com/owner/repo/pull/42", - }}, - }, - } - - tmpDir := t.TempDir() - writeStackFile(t, tmpDir, s) - - restore := git.SetOps(newMergeMock(tmpDir, "feat-1")) - defer restore() - - // NewTestConfig is non-interactive (piped output), so no confirm prompt. - cfg, _, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{ - FindPRByNumberFn: func(number int) (*github.PullRequest, error) { - if number == 42 { - return &github.PullRequest{ - Number: 42, - ID: "PR_42", - URL: "https://github.com/owner/repo/pull/42", - State: "OPEN", - }, nil - } - return nil, nil - }, - } - cmd := MergeCmd(cfg) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - err := cmd.Execute() - - cfg.Err.Close() - errOut, _ := io.ReadAll(errR) - output := string(errOut) - - assert.NoError(t, err) - assert.Contains(t, output, "https://github.com/owner/repo/pull/42") -} - -func TestMerge_NoArgs(t *testing.T) { - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "feat-1"}, - }, - } - - tmpDir := t.TempDir() - writeStackFile(t, tmpDir, s) - - restore := git.SetOps(newMergeMock(tmpDir, "feat-1")) - defer restore() - - cfg, _, _ := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{} - cmd := MergeCmd(cfg) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - cmd.SetArgs([]string{"extra-arg", "another"}) - err := cmd.Execute() - - // MaximumNArgs(1) should reject two positional arguments. - assert.Error(t, err) -} - -func TestMerge_ByPRNumber(t *testing.T) { - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "feat-1", PullRequest: &stack.PullRequestRef{ - Number: 42, - URL: "https://github.com/owner/repo/pull/42", - }}, - {Branch: "feat-2", PullRequest: &stack.PullRequestRef{ - Number: 43, - URL: "https://github.com/owner/repo/pull/43", - }}, - }, - } - - tmpDir := t.TempDir() - writeStackFile(t, tmpDir, s) - - // Current branch is feat-2, but we target PR #42 (feat-1) via arg. - restore := git.SetOps(newMergeMock(tmpDir, "feat-2")) - defer restore() - - cfg, _, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{ - FindPRByNumberFn: func(number int) (*github.PullRequest, error) { - switch number { - case 42: - return &github.PullRequest{Number: 42, URL: "https://github.com/owner/repo/pull/42", State: "OPEN"}, nil - case 43: - return &github.PullRequest{Number: 43, URL: "https://github.com/owner/repo/pull/43", State: "OPEN"}, nil - } - return nil, nil - }, - } - cmd := MergeCmd(cfg) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - cmd.SetArgs([]string{"42"}) - err := cmd.Execute() - - cfg.Err.Close() - errOut, _ := io.ReadAll(errR) - output := string(errOut) - - assert.NoError(t, err) - assert.Contains(t, output, "https://github.com/owner/repo/pull/42") -} - -func TestMerge_ByPRURL(t *testing.T) { - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "feat-1", PullRequest: &stack.PullRequestRef{ - Number: 42, - URL: "https://github.com/owner/repo/pull/42", - }}, - }, - } - - tmpDir := t.TempDir() - writeStackFile(t, tmpDir, s) - - restore := git.SetOps(newMergeMock(tmpDir, "feat-1")) - defer restore() - - cfg, _, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{ - FindPRByNumberFn: func(number int) (*github.PullRequest, error) { - if number == 42 { - return &github.PullRequest{Number: 42, URL: "https://github.com/owner/repo/pull/42", State: "OPEN"}, nil - } - return nil, nil - }, - } - cmd := MergeCmd(cfg) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - cmd.SetArgs([]string{"https://github.com/owner/repo/pull/42"}) - err := cmd.Execute() - - cfg.Err.Close() - errOut, _ := io.ReadAll(errR) - output := string(errOut) - - assert.NoError(t, err) - assert.Contains(t, output, "https://github.com/owner/repo/pull/42") -} - -func TestMerge_ByBranchName(t *testing.T) { - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "feat-1", PullRequest: &stack.PullRequestRef{ - Number: 42, - URL: "https://github.com/owner/repo/pull/42", - }}, - }, - } - - tmpDir := t.TempDir() - writeStackFile(t, tmpDir, s) - - restore := git.SetOps(newMergeMock(tmpDir, "main")) - defer restore() - - cfg, _, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{ - FindPRByNumberFn: func(number int) (*github.PullRequest, error) { - if number == 42 { - return &github.PullRequest{Number: 42, URL: "https://github.com/owner/repo/pull/42", State: "OPEN"}, nil - } - return nil, nil - }, - } - cmd := MergeCmd(cfg) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - cmd.SetArgs([]string{"feat-1"}) - err := cmd.Execute() - - cfg.Err.Close() - errOut, _ := io.ReadAll(errR) - output := string(errOut) - - assert.NoError(t, err) - assert.Contains(t, output, "https://github.com/owner/repo/pull/42") -} diff --git a/cmd/root.go b/cmd/root.go index 217016d..65bfe71 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -107,10 +107,6 @@ locally, then push to GitHub to create your stack of PRs.`, linkCmd.GroupID = "remote" root.AddCommand(linkCmd) - mergeCmd := MergeCmd(cfg) - mergeCmd.GroupID = "remote" - root.AddCommand(mergeCmd) - // Navigation commands switchCmd := SwitchCmd(cfg) switchCmd.GroupID = "nav" diff --git a/cmd/root_test.go b/cmd/root_test.go index 25a68a7..8138c7a 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -10,7 +10,7 @@ import ( func TestRootCmd_SubcommandRegistration(t *testing.T) { root := RootCmd() - expected := []string{"init", "add", "checkout", "push", "sync", "unstack", "merge", "view", "rebase", "up", "down", "top", "bottom", "alias", "feedback", "submit"} + expected := []string{"init", "add", "checkout", "push", "sync", "unstack", "view", "rebase", "up", "down", "top", "bottom", "alias", "feedback", "submit"} registered := make(map[string]bool) for _, cmd := range root.Commands() {