From df3dac04f66c3715dcf89015b587b55505992459 Mon Sep 17 00:00:00 2001 From: csvenke Date: Fri, 6 Mar 2026 12:40:53 +0100 Subject: [PATCH] refactor: remove EnsureBranchPushed functionality - Remove EnsureBranchPushed function from gh package - Remove EnsureBranchPushedFunc variable and its usage from pr command - Remove runGitCommand helper function - Remove all related tests for EnsureBranchPushed --- internal/cmd/cmd.go | 8 +-- internal/cmd/cmd_test.go | 14 ----- internal/cmd/gh/pr/command.go | 9 +--- internal/gh/gh.go | 36 ------------- internal/gh/gh_test.go | 97 ----------------------------------- main.go | 2 +- 6 files changed, 7 insertions(+), 159 deletions(-) diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index 2973655..a115fb7 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -96,8 +96,8 @@ func Run(ctx context.Context, provider providers.Provider, stdout, stderr io.Wri } func (r *Registry) UsageTo(w io.Writer) { - fmt.Fprintf(w, "Usage: llm [options]\n\n") - fmt.Fprintf(w, "Commands:\n") + _, _ = fmt.Fprintf(w, "Usage: llm [options]\n\n") + _, _ = fmt.Fprintf(w, "Commands:\n") r.writeCommands(w, r.commands, 2) } @@ -150,9 +150,9 @@ func findCommand(commands []*Command, name string) *Command { func (r *Registry) writeCommands(w io.Writer, commands []*Command, indent int) { for _, cmd := range commands { if indent <= 2 { - fmt.Fprintf(w, "%s%-19s %s\n", spaces(indent), cmd.Usage, cmd.Description) + _, _ = fmt.Fprintf(w, "%s%-19s %s\n", spaces(indent), cmd.Usage, cmd.Description) } else { - fmt.Fprintf(w, "%s%-17s %s\n", spaces(indent), cmd.Usage, cmd.Description) + _, _ = fmt.Fprintf(w, "%s%-17s %s\n", spaces(indent), cmd.Usage, cmd.Description) } if len(cmd.Subcommands) > 0 { diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index f45a32c..2e0ec82 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -137,11 +137,9 @@ func TestRunRoutesAskAndCommit(t *testing.T) { func TestRunGHSubcommands(t *testing.T) { originalGHRun := prcmd.RunFunc originalGHCreatePullRequest := prcmd.CreatePullRequestFunc - originalEnsureBranchPushed := prcmd.EnsureBranchPushedFunc t.Cleanup(func() { prcmd.RunFunc = originalGHRun prcmd.CreatePullRequestFunc = originalGHCreatePullRequest - prcmd.EnsureBranchPushedFunc = originalEnsureBranchPushed }) deps := Dependencies{ @@ -157,7 +155,6 @@ func TestRunGHSubcommands(t *testing.T) { args []string runErr error createErr error - pushErr error createOutput string wantErr bool wantErrSubstr string @@ -196,13 +193,6 @@ func TestRunGHSubcommands(t *testing.T) { wantErr: true, wantErrSubstr: "generation failed", }, - { - name: "returns push error", - args: []string{"gh", "pr"}, - pushErr: errors.New("push failed"), - wantErr: true, - wantErrSubstr: "push failed", - }, { name: "returns create error", args: []string{"gh", "pr"}, @@ -235,10 +225,6 @@ func TestRunGHSubcommands(t *testing.T) { return tt.createOutput, nil } - prcmd.EnsureBranchPushedFunc = func(client gh.Client) error { - return tt.pushErr - } - err := defaultRegistry.Run(context.Background(), deps, tt.args) if (err != nil) != tt.wantErr { diff --git a/internal/cmd/gh/pr/command.go b/internal/cmd/gh/pr/command.go index 6db8617..45222ad 100644 --- a/internal/cmd/gh/pr/command.go +++ b/internal/cmd/gh/pr/command.go @@ -16,9 +16,8 @@ const ( ) var ( - RunFunc = gh.Run - CreatePullRequestFunc = gh.CreatePullRequest - EnsureBranchPushedFunc = gh.EnsureBranchPushed + RunFunc = gh.Run + CreatePullRequestFunc = gh.CreatePullRequest ) func Run(ctx context.Context, provider providers.Provider, client gh.Client, stdout, stderr io.Writer, args []string) error { @@ -31,10 +30,6 @@ func Run(ctx context.Context, provider providers.Provider, client gh.Client, std return err } - if err := EnsureBranchPushedFunc(client); err != nil { - return err - } - output, err := CreatePullRequestFunc(pr) if err != nil { return err diff --git a/internal/gh/gh.go b/internal/gh/gh.go index 91e9cbe..5482fcb 100644 --- a/internal/gh/gh.go +++ b/internal/gh/gh.go @@ -29,18 +29,6 @@ var ( return "", fmt.Errorf("gh %s: %s", strings.Join(args, " "), strings.TrimSpace(stderr.String())) } - return strings.TrimSpace(stdout.String()), nil - } - runGitCommand = func(args ...string) (string, error) { - cmd := execCommand("git", args...) - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - if err := cmd.Run(); err != nil { - return "", fmt.Errorf("git %s: %s", strings.Join(args, " "), strings.TrimSpace(stderr.String())) - } - return strings.TrimSpace(stdout.String()), nil } ) @@ -114,30 +102,6 @@ func CreatePullRequest(pr *PullRequest) (string, error) { return output, nil } -func EnsureBranchPushed(git Client) error { - if _, err := runGitCommand("rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"); err == nil { - if _, err := runGitCommand("push"); err != nil { - return fmt.Errorf("pushing current branch: %w", err) - } - return nil - } - - branch, err := git.GetCurrentBranch() - if err != nil { - return fmt.Errorf("getting current branch for push: %w", err) - } - - if strings.TrimSpace(branch) == "" { - return fmt.Errorf("unable to determine current branch for push") - } - - if _, err := runGitCommand("push", "-u", "origin", branch); err != nil { - return fmt.Errorf("pushing current branch to origin: %w", err) - } - - return nil -} - func getBaseDiffContext(git Client) (string, string, string, error) { base, mergeBase, err := resolveBaseBranch(git) if err != nil { diff --git a/internal/gh/gh_test.go b/internal/gh/gh_test.go index 1a1f83c..88eb29b 100644 --- a/internal/gh/gh_test.go +++ b/internal/gh/gh_test.go @@ -385,100 +385,3 @@ func TestCreatePullRequest(t *testing.T) { }) } } - -func TestEnsureBranchPushed(t *testing.T) { - originalRunGitCommand := runGitCommand - t.Cleanup(func() { - runGitCommand = originalRunGitCommand - }) - - tests := []struct { - name string - git *stubGitClient - runGit func(args ...string) (string, error) - wantErr bool - wantErrSubstr string - wantCalls [][]string - }{ - { - name: "pushes tracked branch with git push", - git: &stubGitClient{branch: "feature/test"}, - runGit: func(args ...string) (string, error) { - return "", nil - }, - wantCalls: [][]string{ - {"rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"}, - {"push"}, - }, - }, - { - name: "pushes untracked branch to origin with upstream", - git: &stubGitClient{branch: "feature/test"}, - runGit: func(args ...string) (string, error) { - if reflect.DeepEqual(args, []string{"rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"}) { - return "", errors.New("no upstream") - } - return "", nil - }, - wantCalls: [][]string{ - {"rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"}, - {"push", "-u", "origin", "feature/test"}, - }, - }, - { - name: "fails when push errors", - git: &stubGitClient{branch: "feature/test"}, - runGit: func(args ...string) (string, error) { - if reflect.DeepEqual(args, []string{"push"}) { - return "", errors.New("push failed") - } - return "", nil - }, - wantErr: true, - wantErrSubstr: "pushing current branch", - wantCalls: [][]string{ - {"rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"}, - {"push"}, - }, - }, - { - name: "fails when branch is unknown for upstream setup", - git: &stubGitClient{branch: ""}, - runGit: func(args ...string) (string, error) { - if reflect.DeepEqual(args, []string{"rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"}) { - return "", errors.New("no upstream") - } - return "", nil - }, - wantErr: true, - wantErrSubstr: "unable to determine current branch", - wantCalls: [][]string{ - {"rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"}, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var gotCalls [][]string - runGitCommand = func(args ...string) (string, error) { - gotCalls = append(gotCalls, append([]string(nil), args...)) - return tt.runGit(args...) - } - - err := EnsureBranchPushed(tt.git) - - if (err != nil) != tt.wantErr { - t.Fatalf("EnsureBranchPushed() error = %v, wantErr %v", err, tt.wantErr) - } - - if tt.wantErr && tt.wantErrSubstr != "" && (err == nil || !strings.Contains(err.Error(), tt.wantErrSubstr)) { - t.Fatalf("EnsureBranchPushed() error = %v, want substring %q", err, tt.wantErrSubstr) - } - - if !reflect.DeepEqual(gotCalls, tt.wantCalls) { - t.Fatalf("EnsureBranchPushed() calls = %v, want %v", gotCalls, tt.wantCalls) - } - }) - } -} diff --git a/main.go b/main.go index 060ca4f..c4f9960 100644 --- a/main.go +++ b/main.go @@ -72,7 +72,7 @@ func usage() { func usageTo(w io.Writer) { cmd.UsageTo(w) - fmt.Fprintf(w, "\nOptions:\n") + _, _ = fmt.Fprintf(w, "\nOptions:\n") oldOutput := flag.CommandLine.Output() flag.CommandLine.SetOutput(w) defer flag.CommandLine.SetOutput(oldOutput)