Feature 101/GitHub import #104

noe.trevino · 2026-02-18T00:44:43-06:00

noe.trevino commented

2026-02-18 00:44:43 -06:00

Checklist

Read CONTRIBUTING.md
Ran tests (go test ./... -race) locally
Formatted and linted with gofmt -w . and golangci-lint run ./... respectively
Added tests if necessary (significant changes/complex logic)
Updated documentation if applicable
Updated README.md if applicable

Summary

Closes #101

Adds github import command with a spinner.
Not adding into the tui until we do the jira one as will.
When that happens, we can add a command or something

## Checklist - [x] Read `CONTRIBUTING.md` - [x] Ran tests (`go test ./... -race`) locally - [x] Formatted and linted with `gofmt -w .` and `golangci-lint run ./...` respectively - [x] Added tests if necessary (significant changes/complex logic) - [x] Updated documentation if applicable - [x] Updated `README.md` if applicable ## Summary  Closes #101 Adds github import command with a spinner. Not adding into the tui until we do the jira one as will. When that happens, we can add a <c-i> command or something

github-actions[bot] commented

2026-02-18 00:44:55 -06:00

(Migrated from github.com)

Informational Warning:

The snapshot tests have been changed.
Please add a screenshot showing what has been changed, and a description of why it was changed.

If you do not provide an explanation and screenshot in 3 days, this PR will be closed.

@TheNoeTrevino

Changed snapshot files (1 files)

internal/testing/snapshots/cli/testdata/e2e/task-show.golden

**Informational Warning:** The snapshot tests have been changed. Please add a screenshot showing what has been changed, and a description of why it was changed. If you do not provide an explanation and screenshot in 3 days, this PR will be closed. @TheNoeTrevino <details> <summary>Changed snapshot files (1 files)</summary> - `internal/testing/snapshots/cli/testdata/e2e/task-show.golden` </details>

👍 1

claude[bot] (Migrated from github.com) reviewed

2026-02-18 00:45:41 -06:00

internal/cli/gh/import.go

					
				@ -0,0 +118,4 @@

						targetColumnID = col.ID

					}

					issue, err := cliInstance.App.GitHubFetcher.FetchIssue(ctx, issueNumber)

Dead code — cobra.ExactArgs(1) on the command definition (line 104) means cobra rejects the call before Execute is reached if no argument is provided. This check can never be true.

	issueNumber, err := ValidateIssueNumber(args.Args[0])

Dead code — `cobra.ExactArgs(1)` on the command definition (line 104) means cobra rejects the call before `Execute` is reached if no argument is provided. This check can never be true. ```suggestion issueNumber, err := ValidateIssueNumber(args.Args[0]) ```

claude[bot] (Migrated from github.com) reviewed

2026-02-18 00:46:20 -06:00

internal/cli/gh/import.go

					
				@ -0,0 +175,4 @@

				func parseImportFlags(_ *cobra.Command) error {

					return nil

				}

resolveDefaultAuthor() calls config.Load() (a file read) on every comment with an empty author. Hoist this before the loop so the config is read at most once.

`resolveDefaultAuthor()` calls `config.Load()` (a file read) on every comment with an empty author. Hoist this before the loop so the config is read at most once.

claude[bot] (Migrated from github.com) reviewed

2026-02-18 00:46:34 -06:00

internal/cli/gh/import.go

					
				@ -0,0 +136,4 @@

					defaultAuthor := resolveDefaultAuthor()

					commentsImported := 0

					for _, comment := range issue.Comments {

resolveDefaultAuthor() calls config.Load() (a file read) on every comment iteration where the author is empty. Hoist the call before the loop so config is read at most once regardless of how many comments have empty authors.

`resolveDefaultAuthor()` calls `config.Load()` (a file read) on every comment iteration where the author is empty. Hoist the call before the loop so config is read at most once regardless of how many comments have empty authors.

claude[bot] (Migrated from github.com) reviewed

2026-02-18 00:46:50 -06:00

internal/github/github_test.go

					
				@ -0,0 +135,4 @@

					assert.Empty(t, issue.Comments)

				}

				func TestCheckInstalled_GHExists(t *testing.T) {

This test assumes gh is on PATH in every environment. If gh is absent (e.g. a minimal CI image) the test fails spuriously. Either add a skip guard (if _, err := exec.LookPath("gh"); err != nil { t.Skip("gh not installed") }) or mock lookPath as TestCheckInstalled_GHMissing does.

This test assumes `gh` is on `PATH` in every environment. If `gh` is absent (e.g. a minimal CI image) the test fails spuriously. Either add a skip guard (`if _, err := exec.LookPath("gh"); err != nil { t.Skip("gh not installed") }`) or mock `lookPath` as `TestCheckInstalled_GHMissing` does.

claude[bot] commented

2026-02-18 00:46:59 -06:00

(Migrated from github.com)

Review Summary

Three concrete issues found:

1. Dead guard (import.go:120–122)
The len(args.Args) == 0 check inside Execute is unreachable. cobra.ExactArgs(1) already rejects the command before Execute is called when the argument is missing. Remove the guard.

2. Repeated config.Load() inside comment loop (import.go ~line 137)
resolveDefaultAuthor() calls config.Load() (a file read) once per comment with an empty author. Hoist the call above the loop: defaultAuthor := resolveDefaultAuthor().

3. TestCheckInstalled_GHExists has a hard environment dependency (github_test.go ~line 138)
The test uses the real exec.LookPath and will fail in any environment without gh on PATH. Add a skip guard or mock lookPath consistently with TestCheckInstalled_GHMissing.

Score: 8/10

## Review Summary Three concrete issues found: **1. Dead guard (`import.go:120–122`)** The `len(args.Args) == 0` check inside `Execute` is unreachable. `cobra.ExactArgs(1)` already rejects the command before `Execute` is called when the argument is missing. Remove the guard. **2. Repeated `config.Load()` inside comment loop (`import.go` ~line 137)** `resolveDefaultAuthor()` calls `config.Load()` (a file read) once per comment with an empty author. Hoist the call above the loop: `defaultAuthor := resolveDefaultAuthor()`. **3. `TestCheckInstalled_GHExists` has a hard environment dependency (`github_test.go` ~line 138)** The test uses the real `exec.LookPath` and will fail in any environment without `gh` on `PATH`. Add a skip guard or mock `lookPath` consistently with `TestCheckInstalled_GHMissing`. --- **Score: 8/10**

Sign in to join this conversation.

No reviewers