add version command to kubestr#378
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds build-time version injection via ldflags in the release config and a new CLI Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 36 minutes and 58 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/rootCmd.go`:
- Around line 160-166: Add an args validator and use Cobra's output on the
version command: update the versionCmd declaration to include Args: cobra.NoArgs
to reject extra operands, and replace the direct fmt.Println call in the Run
function with cmd.Println(GetVersion()) so output respects Cobra's configured
output stream; locate this change in the versionCmd definition and the Run
closure where GetVersion() is printed.
- Around line 577-601: The version logic currently ignores info.Main.Version
from debug.ReadBuildInfo(), causing binaries built via "go install
module@version" to show "dev"; update the function that calls
debug.ReadBuildInfo() to first check info.Main.Version (from the returned info
variable) and if it's non-empty and not the "(devel)" sentinel, return that
module version string immediately (before scanning info.Settings for
vcs.revision/vcs.modified); keep the existing VCS-based short SHA logic as a
fallback if info.Main.Version is empty, and still return "dev" only when neither
info.Main.Version nor VCS info are available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d14fe7ae-b67f-4978-8297-f8df078c5c76
📒 Files selected for processing (2)
.goreleaser.ymlcmd/rootCmd.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cmd/rootCmd.go (1)
578-602:⚠️ Potential issue | 🟠 Major
GetVersion()still missesgo install module@versionbuilds.At Line 578+, the logic only inspects
info.Settings. For binaries installed viago install ...@vX.Y.Z,info.Main.Versionis typically the authoritative value and VCS settings may be absent, so this can incorrectly return"dev".Suggested fix
info, ok := debug.ReadBuildInfo() if !ok { // No build info available (e.g. built outside a git repo). return "dev" } + if v := info.Main.Version; v != "" && v != "(devel)" { + return v + } var revision, modified string for _, s := range info.Settings {#!/bin/bash # Verify current GetVersion logic does not check info.Main.Version before VCS fallback. rg -n 'ReadBuildInfo|info\.Main\.Version|vcs\.revision|vcs\.modified' cmd/rootCmd.go -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/rootCmd.go` around lines 578 - 602, GetVersion currently only reads info.Settings for vcs.revision/vcs.modified and returns "dev" when those are absent; update GetVersion (the function using debug.ReadBuildInfo and info.Settings) to first check info.Main.Version and if it is non-empty and not "(devel)" return that version (so go install module@vX.Y.Z builds are respected), otherwise fall back to the existing loop over info.Settings to derive a short vcs.revision plus optional -dirty and then the final "dev" fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cmd/rootCmd.go`:
- Around line 578-602: GetVersion currently only reads info.Settings for
vcs.revision/vcs.modified and returns "dev" when those are absent; update
GetVersion (the function using debug.ReadBuildInfo and info.Settings) to first
check info.Main.Version and if it is non-empty and not "(devel)" return that
version (so go install module@vX.Y.Z builds are respected), otherwise fall back
to the existing loop over info.Settings to derive a short vcs.revision plus
optional -dirty and then the final "dev" fallback.
| // If the version was injected at build time via ldflags by goreleaser, it returns that. | ||
| // Otherwise, it falls back to reading the git commit hash that Go automatically | ||
| // bakes into the binary since Go 1.18 (see runtime/debug.BuildSetting). | ||
| func GetVersion() string { |
There was a problem hiding this comment.
@adamMolner does this function need to be exported? or could it be "unexported"?
| func GetVersion() string { | |
| func getVersion() string { |
There was a problem hiding this comment.
| ) | ||
|
|
||
| var ( | ||
| version = "dev" // overridden at build time via ldflags |
There was a problem hiding this comment.
This could be empty by default, since the fallback would be to get it from the runtime when it is not explicitly set.
| version = "dev" // overridden at build time via ldflags | |
| version string // overridden at build time via ldflags |
There was a problem hiding this comment.
| func GetVersion() string { | ||
| // version is set at build time via ldflags by goreleaser (e.g. v0.4.49). | ||
| // If it was injected, return it. | ||
| if version != "dev" { |
There was a problem hiding this comment.
| if version != "dev" { | |
| if version != "" { |
There was a problem hiding this comment.
| } | ||
| } | ||
| if revision != "" { | ||
| return fmt.Sprintf("dev (%s%s)", revision, modified) |
There was a problem hiding this comment.
maybe modify the format so it does not contain spaces?
There was a problem hiding this comment.
There was a problem hiding this comment.
Pull request overview
Adds a kubestr version CLI command and wires release builds to inject a version string via GoReleaser, with a development fallback that reads VCS build info (revision + dirty state) from Go’s embedded build metadata.
Changes:
- Introduces a
versionsubcommand that prints a computed version string. - Adds
GetVersion()to return either an ldflags-injected release version or a dev version derived from embedded VCS metadata. - Updates GoReleaser build configuration to set
cmd.versionat link time.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cmd/rootCmd.go | Adds version command, version build variable, and GetVersion() fallback logic based on debug.ReadBuildInfo(). |
| .goreleaser.yml | Injects build-time version into cmd.version using -ldflags -X .... |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| versionCmd = &cobra.Command{ | ||
| Use: "version", | ||
| Short: "Print the version of kubestr", | ||
| Args: cobra.NoArgs, |
There was a problem hiding this comment.
versionCmd uses cobra.NoArgs while other commands in this file consistently use cobra.ExactArgs(0) for no-arg commands (e.g., fioCmd, csiCheckCmd, blockMountCmd). Consider switching to cobra.ExactArgs(0) for consistency across commands.
| Args: cobra.NoArgs, | |
| Args: cobra.ExactArgs(0), |
There was a problem hiding this comment.
I originally had cobra.ExactArgs(0), but coderabbit said change it to NoArgs, but then copilot said change it back to match our conventions.
| // version is set at build time via ldflags by goreleaser (e.g. v0.4.49). | ||
| // If it was injected, return it. | ||
| if version != "dev" { | ||
| return version | ||
| } |
There was a problem hiding this comment.
GetVersion() returns version for any value other than "dev"; if the ldflag is accidentally set to an empty string, kubestr version will print a blank line. Consider treating empty string as unset (e.g., only returning version when it is non-empty and not "dev").
| ldflags: | ||
| - -X github.com/kastenhq/kubestr/cmd.version={{.Version}} |
There was a problem hiding this comment.
This injects {{.Version}} into cmd.version, but GoReleaser templates typically set .Version to the semver without a leading "v" (while .Tag includes it). If the intention is for kubestr version to print vX.Y.Z as shown in the PR description, consider injecting {{.Tag}} or prefixing v explicitly.
There was a problem hiding this comment.
The ldflag that injects the version string into the binary went from {{.Version}} to v{{ trimprefix .Version "v" }}. Goreleaser's .Version is the git tag with the leading v stripped (so v0.4.49 becomes 0.4.49), which meant kubestr version would print 0.4.49 for real releases — missing the v that's on the tag and release page. The new template strips any leading v and re-adds it, guaranteeing exactly one v prefix in all build modes (real release, snapshot, etc.) and avoiding the double-vv you'd get if you naively prepended v to a value that already had one.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cmd/rootCmd.go (1)
160-166:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse Cobra’s output writer here.
fmt.Println()bypasses the command’s configured streams, sokubestr versionis harder to test and won’t honor redirected output. Switch this tocmd.Println(getVersion()).Suggested fix
versionCmd = &cobra.Command{ Use: "version", Short: "Print the version of kubestr", Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { - fmt.Println(getVersion()) + cmd.Println(getVersion()) }, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/rootCmd.go` around lines 160 - 166, The version command is printing via fmt.Println which bypasses Cobra's output streams; update the Run function of versionCmd to write using the command's output writer by replacing fmt.Println(getVersion()) with cmd.Println(getVersion()) so the getVersion() result goes through Cobra's configured streams (use the cmd parameter available in the Run func).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cmd/rootCmd.go`:
- Around line 160-166: The version command is printing via fmt.Println which
bypasses Cobra's output streams; update the Run function of versionCmd to write
using the command's output writer by replacing fmt.Println(getVersion()) with
cmd.Println(getVersion()) so the getVersion() result goes through Cobra's
configured streams (use the cmd parameter available in the Run func).
addresses issue: #138
This PR adds a
versioncommand to kubestr.It does two things:
versionvariable tocmd/rootCmd.gothat defaults to"dev"and gets overwritten at build time via-ldflagswhen goreleaser builds an official releasego build ., falls back to reading the git commit hash that Go automatically bakes into the binary since Go 1.18 (seeruntime/debug.BuildSetting) — this way dev builds still show something useful. A-dirtysuffix is appended if there were uncommitted changes at build time, following the convention established bygit describe --dirtyThe behavior looks like this depending on how the binary was built:
Unfortunately there is no single approach that works for all cases since go build and go install behave differently with respect to version stamping. The hybrid approach here covers all realistic scenarios without requiring changes to the release pipeline beyond adding the -X ldflag.
Summary by CodeRabbit