chore: initial setup Go CLI with Cobra framework#2
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughInitializes a Go Cobra CLI: adds module config and .gitignore, a CLI entry point, Cobra root and version commands, a version package constant, and unit tests for main and command behaviors. Changes
Sequence DiagramsequenceDiagram
participant User as User/Shell
participant Main as main.go
participant Execute as cmd.Execute()
participant RootCmd as rootCmd
participant VersionCmd as versionCmd
participant VersionPkg as version.Version
User->>Main: run CLI executable
Main->>Execute: Invoke Execute()
Execute->>RootCmd: Execute rootCmd
Note over RootCmd: parse args & route
User->>RootCmd: "version" arg
RootCmd->>VersionCmd: dispatch to version
VersionCmd->>VersionPkg: read Version
VersionPkg-->>VersionCmd: return "0.1.0-next"
VersionCmd-->>User: print version output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/root.go`:
- Around line 15-19: The Execute function currently prints errors manually which
duplicates Cobra's own error output; in the Execute() function remove the
explicit fmt.Fprintln(os.Stderr, err) call (leave the err check and os.Exit(1)
so the process still exits on error) and rely on rootCmd/Cobra to handle error
printing.
In `@cmd/version.go`:
- Around line 11-16: The version command currently uses fmt.Printf and accepts
any args; change the cobra command definition for versionCmd to enforce no args
by adding Args: cobra.NoArgs and switch the output call inside the Run function
from fmt.Printf(...) to cmd.Printf(...) so the command validates unexpected
arguments and uses Cobra's configured output writer (reference symbols:
versionCmd, Args: cobra.NoArgs, Run func, cmd.Printf vs fmt.Printf).
In `@go.mod`:
- Around line 3-10: The go.mod declares "go 1.25.7" but CI/workflows may use a
different Go version; update your CI to explicitly install/use Go 1.25.7 so
builds/tests match the module setting—e.g., set the setup action or Docker image
to use go-version 1.25.7 in the workflow(s) that run builds/tests and update any
matrix entries that reference other Go versions to include 1.25.7; if the repo
uses container images or Makefile toolchains, ensure those are aligned as well
so "go 1.25.7" in go.mod is enforced across all CI runs.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
.gitignorecmd/root.gocmd/version.gogo.modmain.go
- Initialize module as github.com/kortex-hub/kortex-cli - Add root command for launching and managing AI agents - Add version command (0.1.0-next) - Configure Cobra command structure - Add .gitignore for Go build artifacts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Update version command to reject unexpected arguments and use Cobra's output writer for better testability and framework integration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com> Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com> Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/cmd/root.go (1)
20-22: Remove emptyinit()until needed.This keeps the file tighter and avoids dead placeholder hooks.
Optional cleanup
-func init() { - // Global flags can be added here -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/root.go` around lines 20 - 22, Remove the empty init() function defined as init() — delete the entire empty init() block (the no-op global initializer) so there are no dead placeholder hooks; confirm there are no side effects or dependencies expecting this init() before removing, then run `go vet`/build to ensure nothing breaks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/cmd/root.go`:
- Around line 20-22: Remove the empty init() function defined as init() — delete
the entire empty init() block (the no-op global initializer) so there are no
dead placeholder hooks; confirm there are no side effects or dependencies
expecting this init() before removing, then run `go vet`/build to ensure nothing
breaks.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
.gitignorecmd/kortex-cli/main.gogo.modpkg/cmd/root.gopkg/cmd/version.gopkg/version/version.go
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- go.mod
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/kortex-cli/main_test.go (1)
8-18: Consider capturing stdout to verify actual output.This smoke test correctly saves/restores
os.Argsand verifies no panic occurs, which is a reasonable starting point. However, the test would pass even if the version subcommand printed nothing or incorrect output.For more robust coverage, consider capturing stdout and asserting on the expected version string:
♻️ Suggested improvement with output verification
package main import ( + "bytes" + "io" "os" + "strings" "testing" ) func TestMain_VersionSubcommand(t *testing.T) { // Save original os.Args and restore after test oldArgs := os.Args defer func() { os.Args = oldArgs }() + // Capture stdout + oldStdout := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + // Set os.Args to call the version subcommand os.Args = []string{"kortex-cli", "version"} - // Call main() - test passes if it doesn't panic + // Call main() main() + + // Restore stdout and read captured output + w.Close() + os.Stdout = oldStdout + var buf bytes.Buffer + io.Copy(&buf, r) + + // Verify output contains version info + if !strings.Contains(buf.String(), "0.1.0") { + t.Errorf("expected version output to contain '0.1.0', got: %s", buf.String()) + } }Note: If any code path (e.g., error handling in
Execute()) callsos.Exit(), it will terminate the test process entirely. This is a common Go testing caveat—consider verifying that successful commands don't invokeos.Exit().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/kortex-cli/main_test.go` around lines 8 - 18, Update TestMain_VersionSubcommand to capture and assert stdout rather than only checking no panic: create a pipe or use bytes.Buffer to redirect os.Stdout, run the same os.Args setup and call main() (or the command entry like Execute()/rootCmd.Execute() if available), restore os.Stdout, and assert the captured output contains the expected version string; also ensure the test avoids os.Exit terminating the process by invoking the command entrypoint that returns an error instead of exiting or by mocking/stubbing any code path that calls os.Exit().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/kortex-cli/main_test.go`:
- Around line 8-18: Update TestMain_VersionSubcommand to capture and assert
stdout rather than only checking no panic: create a pipe or use bytes.Buffer to
redirect os.Stdout, run the same os.Args setup and call main() (or the command
entry like Execute()/rootCmd.Execute() if available), restore os.Stdout, and
assert the captured output contains the expected version string; also ensure the
test avoids os.Exit terminating the process by invoking the command entrypoint
that returns an error instead of exiting or by mocking/stubbing any code path
that calls os.Exit().
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
benoitf
left a comment
There was a problem hiding this comment.
looks OK, only missing copyright headers
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/cmd/root_test.go (1)
35-69: These execution tests should assert output and share a helper.Lines 50 and 68 call
Execute()and only verify “no panic”; they don’t validate command behavior. Also, the arg/output setup is duplicated. Consider a small helper that executesrootCmdwithSetArgs, returns output, and lets each test assert expected text (e.g., usage/help).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/root_test.go` around lines 35 - 69, Replace the duplicated os.Args and output-redirection logic in TestExecute_WithHelp and TestExecute_NoArgs with a small helper (e.g., runRootCmd(args []string) string) that uses rootCmd.SetArgs(args), captures rootCmd.Out via SetOut to a bytes.Buffer, calls Execute() (or rootCmd.Execute() if more appropriate), restores output, and returns buf.String(); then update each test to call this helper and assert the returned string contains the expected text (e.g., "help" or "Usage") instead of only ensuring no panic; reference TestExecute_WithHelp, TestExecute_NoArgs, rootCmd, Execute(), rootCmd.SetArgs and rootCmd.SetOut when making the changes.pkg/cmd/version_test.go (1)
39-55: Strengthen this test to assert behavior and avoidos.Exitpath.Line 54 calls
Execute()(the wrapper inpkg/cmd/root.go) which exits the process on error, and the test currently doesn’t assert the version output. Prefer executingrootCmddirectly with test args and asserting emitted content.✅ Suggested refactor
import ( "bytes" - "os" + "strings" "testing" ) @@ func TestExecute_WithVersion(t *testing.T) { - // Save original os.Args and restore after test - oldArgs := os.Args - defer func() { os.Args = oldArgs }() - - // Set os.Args to call version subcommand - os.Args = []string{"kortex-cli", "version"} - - // Redirect output to avoid cluttering test output + // Redirect output and execute command directly oldStdout := rootCmd.OutOrStdout() + oldStderr := rootCmd.ErrOrStderr() buf := new(bytes.Buffer) rootCmd.SetOut(buf) + rootCmd.SetErr(buf) + rootCmd.SetArgs([]string{"version"}) - defer rootCmd.SetOut(oldStdout) + defer func() { + rootCmd.SetOut(oldStdout) + rootCmd.SetErr(oldStderr) + rootCmd.SetArgs(nil) + }() - // Call Execute() - test passes if it doesn't panic - Execute() + if err := rootCmd.Execute(); err != nil { + t.Fatalf("unexpected execute error: %v", err) + } + if !strings.Contains(buf.String(), "0.1.0-next") { + t.Fatalf("expected version output, got: %q", buf.String()) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/version_test.go` around lines 39 - 55, Update TestExecute_WithVersion to avoid calling the Execute() wrapper (which can os.Exit) and instead run the cobra rootCmd directly: set rootCmd.SetOut(buf) and rootCmd.SetArgs([]string{"version"}), call rootCmd.ExecuteC(), assert the call returns no error, and assert the captured buffer (buf.String()) contains the expected version substring; keep restoring any modified state (original os.Args, rootCmd output) and reference TestExecute_WithVersion, Execute(), and rootCmd in your changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/cmd/root_test.go`:
- Around line 35-69: Replace the duplicated os.Args and output-redirection logic
in TestExecute_WithHelp and TestExecute_NoArgs with a small helper (e.g.,
runRootCmd(args []string) string) that uses rootCmd.SetArgs(args), captures
rootCmd.Out via SetOut to a bytes.Buffer, calls Execute() (or rootCmd.Execute()
if more appropriate), restores output, and returns buf.String(); then update
each test to call this helper and assert the returned string contains the
expected text (e.g., "help" or "Usage") instead of only ensuring no panic;
reference TestExecute_WithHelp, TestExecute_NoArgs, rootCmd, Execute(),
rootCmd.SetArgs and rootCmd.SetOut when making the changes.
In `@pkg/cmd/version_test.go`:
- Around line 39-55: Update TestExecute_WithVersion to avoid calling the
Execute() wrapper (which can os.Exit) and instead run the cobra rootCmd
directly: set rootCmd.SetOut(buf) and rootCmd.SetArgs([]string{"version"}), call
rootCmd.ExecuteC(), assert the call returns no error, and assert the captured
buffer (buf.String()) contains the expected version substring; keep restoring
any modified state (original os.Args, rootCmd output) and reference
TestExecute_WithVersion, Execute(), and rootCmd in your changes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cmd/kortex-cli/main.gocmd/kortex-cli/main_test.gopkg/cmd/root.gopkg/cmd/root_test.gopkg/cmd/version.gopkg/cmd/version_test.gopkg/version/version.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/cmd/version.go
- pkg/cmd/root.go
- cmd/kortex-cli/main_test.go
- cmd/kortex-cli/main.go
Co-authored-by: Jeff MAURY <jmaury@redhat.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Uh oh!
There was an error while loading. Please reload this page.