Enhance keychain configuration and add version check feature - #44
Enhance keychain configuration and add version check feature#44evgenyk wants to merge 6 commits into
Conversation
…structured logging option for improved automation support
…ison and enhance version command output formatting
…version check feature for Kinde CLI
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
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 |
![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
🧹 Nitpick comments (6)
pkg/release/release.go (4)
39-45: Emit structured output for dev builds when KINDE_STRUCTURED_LOG=true.Helps CI consume machine-readable results.
- currentVersion, err := semver.NewVersion(strings.TrimPrefix(Version, "v")) + currentVersion, err := semver.NewVersion(strings.TrimPrefix(Version, "v")) if err != nil { - // Handle development versions or invalid semver - fmt.Printf("Current version '%s' is not a semantic version (development build)\n", Version) - fmt.Printf("Latest release: %v\n", latest) + if structured { + fmt.Printf("{\"status\":\"dev_build\",\"current\":\"%s\",\"latest\":\"%s\"}\n", Version, latest) + } else { + fmt.Printf("Current version '%s' is not a semantic version (development build)\n", Version) + fmt.Printf("Latest release: %v\n", latest) + } return }Add once at function start:
structured := os.Getenv("KINDE_STRUCTURED_LOG") == "true"
47-51: Return structured error on parse failure of latest tag.Keeps CLI usable in automation.
- latestVersion, err := semver.NewVersion(strings.TrimPrefix(latest, "v")) + latestVersion, err := semver.NewVersion(strings.TrimPrefix(latest, "v")) if err != nil { - fmt.Printf("Error parsing latest version '%s': %v\n", latest, err) + if structured { + fmt.Printf("{\"status\":\"error\",\"error\":\"parse_latest\",\"current\":\"%s\",\"latest_raw\":\"%s\"}\n", Version, latest) + } else { + fmt.Printf("Error parsing latest version '%s': %v\n", latest, err) + } return }
10-10: Prepare imports for structured mode.You’ll need
osfor env reads."strings" "time" "github.com/Masterminds/semver/v3" "github.com/briandowns/spinner" "github.com/google/go-github/v28/github" + "os"
53-61: Gate release output byKINDE_STRUCTURED_LOG
- In
pkg/release/release.go, parseKINDE_STRUCTURED_LOGinto astructuredboolean at the top ofIsNeedingUpdate().- Wrap the spinner (and “Checking for updates…”) and the human‐readable
fmt.Printfcalls inif !structured { … } else { … }, emitting JSON in each branch (e.g.{"status":"update_available","current":…,"latest":…},{"status":"latest","current":…},{"status":"ahead","current":…,"latest":…}) whenstructuredis true.- Add unit tests in
pkg/releaseto verify both structured (JSON) and human outputs.pkg/cmd/versionCommand.go (1)
30-31: Show a sensible version for dev builds and support quiet mode.Print “dev” (or commit) when Version is empty; suppress ASCII art when KINDE_STRUCTURED_LOG=true.
- fmt.Printf("Version %v\n", release.Version) + v := release.Version + if v == "" { + v = "dev" + } + fmt.Printf("Version %s\n", v)Additionally (outside selected lines):
// import "os" if os.Getenv("KINDE_STRUCTURED_LOG") == "true" { // skip ASCII art } else { fmt.Print(`...ASCII...`) }README.md (1)
243-246: Add a security caution for KINDE_KEYCHAIN_PASS and clarify structured logs.Environment-stored secrets can leak via process lists, shell history, or CI logs. Recommend a caution and note JSON-only output disables ASCII art/spinners.
- - `KINDE_KEYCHAIN_PASS` - Override the default keychain password when user interaction is not possible + - `KINDE_KEYCHAIN_PASS` - Override the default keychain password when user interaction is not possible (Use with care: treat as a secret; prefer CI secret managers and ephemeral env vars.) - `KINDE_LOG_LEVEL` - Set the logging level for the CLI - - `KINDE_STRUCTURED_LOG` - Enable JSON-only output for CI/CD or automated processing (default: `false`, set to `true` to enable) + - `KINDE_STRUCTURED_LOG` - Enable JSON-only output for CI/CD or automated processing (default: `false`, set to `true` to enable). Disables ASCII art and spinners.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
.vscode/launch.jsonis excluded by!**/*.jsongo.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (3)
README.md(4 hunks)pkg/cmd/versionCommand.go(1 hunks)pkg/release/release.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/cmd/versionCommand.go (1)
pkg/release/release.go (1)
Version(16-16)
🔇 Additional comments (4)
README.md (4)
34-38: Brew upgrade snippet LGTM.Clear and correct.
45-49: Scoop upgrade snippet LGTM.Works as intended.
142-143: Usage entry for version is clear.Matches the command behavior.
146-160: Document structured/quiet behavior ofversionor align code.If the CLI supports
KINDE_STRUCTURED_LOG, note thatversiondisables ASCII art/spinner and emits JSON. If not yet implemented, please align code or adjust docs.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/release/release.go (1)
36-40: Nil-safe tag access implemented; resolves prior concern.Using rep.GetTagName() with an empty guard avoids the nil deref reported earlier. LGTM.
🧹 Nitpick comments (3)
pkg/release/release.go (3)
42-49: Nit: normalize whitespace before parsing current version.This makes parsing resilient to accidental spaces from build-time injection.
- currentVersion, err := semver.NewVersion(strings.TrimPrefix(Version, "v")) + normalizedCurrent := strings.TrimSpace(strings.TrimPrefix(Version, "v")) + currentVersion, err := semver.NewVersion(normalizedCurrent)
51-55: Apply the same normalization to the latest tag.Keeps behavior consistent and robust.
- latestVersion, err := semver.NewVersion(strings.TrimPrefix(latest, "v")) + normalizedLatest := strings.TrimSpace(strings.TrimPrefix(latest, "v")) + latestVersion, err := semver.NewVersion(normalizedLatest)
57-65: Optional: expose a machine-readable status for automation.To better support CI, consider returning a status (or emitting a single structured line) in addition to human text.
-func IsNeedingUpdate() { +// UpdateStatus indicates comparison outcome for automation. +type UpdateStatus int +const ( + UpdateUnknown UpdateStatus = iota + UpdateAvailable + UpdateUpToDate + UpdateAhead +) +func IsNeedingUpdate() UpdateStatus { @@ - fmt.Printf("An update is available: %v (current: %v)\n", latest, Version) + fmt.Printf("An update is available: %v (current: %v)\n", latest, Version) + return UpdateAvailable } else if currentVersion.Equal(latestVersion) { - fmt.Printf("You are using the latest version: %v\n", Version) + fmt.Printf("You are using the latest version: %v\n", Version) + return UpdateUpToDate } else { - fmt.Printf("You are using a newer version than the latest release: %v (latest: %v)\n", Version, latest) + fmt.Printf("You are using a newer version than the latest release: %v (latest: %v)\n", Version, latest) + return UpdateAhead } + return UpdateUnknownAlternatively, emit one line like:
update_status=available|up_to_date|aheadfor easy grepping.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(4 hunks)pkg/release/release.go(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
The merge-base changed after approval.
|
@CodeRabbit re-review please |
|
@evgenyk I'll conduct a fresh review of your PR that enhances keychain configuration and adds the version check feature. ✅ Actions performedReview triggered.
|
The merge-base changed after approval.
The merge-base changed after approval.
The merge-base changed after approval.
The merge-base changed after approval.
The merge-base changed after approval.
2 participants
Update the README to include enhancements for keychain configuration and structured logging. Introduce a version check feature for the Kinde CLI, improving automation support. Update dependencies for version comparison and enhance version command output formatting. Upgrade installation commands for better clarity.