update version generation#346
Merged
Merged
Conversation
Reviewer's GuideUpdate root and self command help/version handling to delegate through Cobra’s built-in mechanisms, centralize version string construction, and relax config requirements for --version, with tests and changelog updated accordingly. Sequence diagram for updated help handling via CobrasequenceDiagram
actor User
participant Main
participant RootCommand
participant Cobra
User->>Main: Main(version, buildDate)
Main->>RootCommand: newRootCmd(version, buildDate)
RootCommand->>RootCommand: SetHelpFunc(customHelpFunc)
Main->>Cobra: rootCmd.Execute()
alt root command help
User->>Cobra: lets
Cobra->>RootCommand: RunE
RootCommand->>RootCommand: cmd.Help()
RootCommand->>RootCommand: customHelpFunc(rootCmd)
RootCommand->>RootCommand: PrintRootHelpMessage(rootCmd)
else subcommand help
User->>Cobra: lets subcommand
Cobra->>RootCommand: RunE
RootCommand->>RootCommand: cmd.Help()
RootCommand->>RootCommand: customHelpFunc(subcommand)
RootCommand->>RootCommand: PrintHelpMessage(subcommand)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
buildDateannotation on the root command is now unused after inlining it intoVersion; consider removing theAnnotations: map[string]string{"buildDate": buildDate}and any remaining references to that annotation to avoid stale state. PrintVersionMessageno longer appears to be used now that--versionis handled via Cobra andcmd.Version; consider deleting it or reusing it viaSetVersionTemplateto reduce unused API surface.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `buildDate` annotation on the root command is now unused after inlining it into `Version`; consider removing the `Annotations: map[string]string{"buildDate": buildDate}` and any remaining references to that annotation to avoid stale state.
- `PrintVersionMessage` no longer appears to be used now that `--version` is handled via Cobra and `cmd.Version`; consider deleting it or reusing it via `SetVersionTemplate` to reduce unused API surface.
## Individual Comments
### Comment 1
<location path="internal/cmd/root.go" line_range="92-95" />
<code_context>
SilenceUsage: true,
}
+
+ cmd.SetHelpFunc(func(c *cobra.Command, _ []string) {
+ var err error
+ if c == c.Root() {
+ err = PrintRootHelpMessage(c)
+ } else {
+ err = PrintHelpMessage(c)
+ }
+
+ if err != nil {
+ c.Println(err)
+ }
+ })
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid printing help errors to stdout; prefer stderr to keep output channels separated.
In the custom help func, errors from `PrintRootHelpMessage` / `PrintHelpMessage` are printed via `c.Println(err)`, so they go to stdout and mix with normal help output, which can break tooling that parses stdout. Since `SetHelpFunc` can’t return errors, consider using `c.PrintErrln(err)` so error output goes to stderr instead.
```suggestion
if err != nil {
c.PrintErrln(err)
}
})
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+92
to
+95
| if err != nil { | ||
| c.Println(err) | ||
| } | ||
| }) |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid printing help errors to stdout; prefer stderr to keep output channels separated.
In the custom help func, errors from PrintRootHelpMessage / PrintHelpMessage are printed via c.Println(err), so they go to stdout and mix with normal help output, which can break tooling that parses stdout. Since SetHelpFunc can’t return errors, consider using c.PrintErrln(err) so error output goes to stderr instead.
Suggested change
| if err != nil { | |
| c.Println(err) | |
| } | |
| }) | |
| if err != nil { | |
| c.PrintErrln(err) | |
| } | |
| }) |
kindermax
force-pushed
the
update-version-generation
branch
2 times, most recently
from
d8d494d to
2c970e4
Compare
kindermax
force-pushed
the
update-version-generation
branch
from
2c970e4 to
e7462b6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Align help and version handling with Cobra defaults while simplifying version formatting and configuration checks.
Bug Fixes:
selfsubcommand delegate to Cobra's help handling when invoked without arguments.--versionflag to bypass configuration requirements so version can be queried without a config file.Enhancements:
Documentation:
Tests:
selfhelp delegation, root command version formatting, and the relaxed config requirement for--version.