Conversation
silv-io
force-pushed
the
silv-io/flc-360
branch
from
247b484 to
7f64203
Compare
silv-io
force-pushed
the
silv-io/flc-360
branch
from
7f64203 to
8489119
Compare
cmd/root.go
Outdated
|
|
||
| if showVersion, err := cmd.Flags().GetBool("version"); err == nil && showVersion { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
suggestion: maybe worth adding a comment here because it took me a while to understand that we want to skip initializing the config when running the version command.
Another option might be to define PersistentPreRunE one level down, per command, so we simply don't call it for version?
Also from what I understood, --version never reaches PersistentPreRunE since it is a built-in flag that prints the template set withSetVersionTemplate, so we could remove the second if?
There was a problem hiding this comment.
I think if we remove the config init from the root command it might introduce more noise to the other commands. I don't see us not needing it anywhere else. I'll add a comment though for why that's needed.
I'll check with the --version tag and remove the other if. Thanks for pointing it out :)
carole-lavillonniere
left a comment
carole-lavillonniere
left a comment
There was a problem hiding this comment.
Left one suggestion, but it looks 🌟
📝 WalkthroughWalkthroughThe changes introduce a release automation system using CalVer versioning. They add GitHub Actions workflows for CI/CD and tag creation, GoReleaser configuration for binary building and distribution, and CLI version display functionality with build metadata embedding. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Git as Git/GitHub
participant GHA as GitHub Actions
participant GoReleaser as GoReleaser
participant Artifacts as Release Artifacts
Dev->>Git: Push tag (format: YYYY.M.patch)
Git->>GHA: Trigger CI workflow on tag push
GHA->>GHA: Run linting & tests
GHA->>GoReleaser: Invoke GoReleaser check
GoReleaser-->>GHA: Validation passed
GHA->>GoReleaser: Build & release with --clean
GoReleaser->>GoReleaser: Embed version/commit/buildDate
GoReleaser->>Artifacts: Generate binaries (tar.gz/zip)
GoReleaser->>Artifacts: Generate checksums
Artifacts-->>Git: Upload release artifacts
sequenceDiagram
participant User as User/Trigger
participant GHA as GitHub Actions
participant Git as Git
participant CLI as CLI Binary
User->>GHA: Trigger create-release-tag workflow
GHA->>Git: Fetch all tags
GHA->>GHA: Compute next CalVer tag (YYYY.M.patch)
GHA->>Git: Validate tag doesn't exist
GHA->>Git: Create & push annotated tag
Git-->>GHA: Tag created
GHA-->>User: Output: tag name
User->>CLI: Run lstk version
CLI-->>User: Display: lstk <version> (<commit>, <buildDate>)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
cmd/root.go (1)
25-28: Name-based check is slightly fragile.The string comparison
cmd.Name() == "version"works for the current command structure but could match unintended commands if a nested subcommand named "version" is added later (e.g.,lstk config version).A more robust approach would compare against the command instance directly, though this requires handling the init order between files.
💡 Alternative approach
- if cmd.Name() == "version" { + if cmd == versionCmd { return nil }This requires ensuring
versionCmdis initialized before this check runs, which is already satisfied since both are in packagecmdinit functions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` around lines 25 - 28, Replace the fragile name-based check (cmd.Name() == "version") with an identity comparison against the actual command instance (versionCmd) to avoid false matches for nested subcommands; update the conditional to return nil when cmd == versionCmd, ensuring versionCmd is initialized before use (both live in package init so init order is satisfied).cmd/version.go (1)
13-16: Consider usingoutput.NewPlainSinkfor consistency.The coding guidelines suggest using
output.NewPlainSink(...)for CLI text output rather than printing directly. However, for a simple single-line version output, the current approach usingcmd.OutOrStdout()is idiomatic Cobra and provides testability.This is a minor consistency consideration - the current implementation works well for this use case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/version.go` around lines 13 - 16, Replace the direct fmt.Fprintln call in the RunE of the version command with output.NewPlainSink to align with CLI output conventions: create a sink using output.NewPlainSink(cmd.OutOrStdout()), write the single-line string returned by versionLine() to that sink, and return any resulting error; reference the RunE anonymous function for the version command and the versionLine() helper when making the change.README.md (1)
1-63: Consider updating CLAUDE.md with the new versioning and release information.This PR introduces significant changes including a new
versioncommand, build process updates with GoReleaser, and architectural changes to the release workflow. Based on learnings, CLAUDE.md should be updated when making such significant changes to help maintain project documentation consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 1 - 63, Update CLAUDE.md to reflect the new versioning and release workflow introduced in this PR: add the CalVer/SemVer format (YYYY.M.patch) and example, document the new version output format used by the CLI (`lstk version` / `lstk --version` -> "lstk <version> (<commit>, <buildDate>)" and the local-dev defaults `version=dev`, `commit=none`, `buildDate=unknown`), outline the GoReleaser-based release steps (mention "Create Release Tag" and "LSTK CI" workflows and that tag pushes trigger the release job), list published artifacts and archive formats for platforms (linux/darwin/windows amd64/arm64) and checksums, and add the local dry-run command (`goreleaser release --snapshot --clean`) so CLAUDE.md matches README.md 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 `@cmd/root.go`:
- Around line 25-28: Replace the fragile name-based check (cmd.Name() ==
"version") with an identity comparison against the actual command instance
(versionCmd) to avoid false matches for nested subcommands; update the
conditional to return nil when cmd == versionCmd, ensuring versionCmd is
initialized before use (both live in package init so init order is satisfied).
In `@cmd/version.go`:
- Around line 13-16: Replace the direct fmt.Fprintln call in the RunE of the
version command with output.NewPlainSink to align with CLI output conventions:
create a sink using output.NewPlainSink(cmd.OutOrStdout()), write the
single-line string returned by versionLine() to that sink, and return any
resulting error; reference the RunE anonymous function for the version command
and the versionLine() helper when making the change.
In `@README.md`:
- Around line 1-63: Update CLAUDE.md to reflect the new versioning and release
workflow introduced in this PR: add the CalVer/SemVer format (YYYY.M.patch) and
example, document the new version output format used by the CLI (`lstk version`
/ `lstk --version` -> "lstk <version> (<commit>, <buildDate>)" and the local-dev
defaults `version=dev`, `commit=none`, `buildDate=unknown`), outline the
GoReleaser-based release steps (mention "Create Release Tag" and "LSTK CI"
workflows and that tag pushes trigger the release job), list published artifacts
and archive formats for platforms (linux/darwin/windows amd64/arm64) and
checksums, and add the local dry-run command (`goreleaser release --snapshot
--clean`) so CLAUDE.md matches README.md changes.
2 participants
Motivation
We need a reliable, tag-driven release pipeline for
lstkusing goreleasesr.For this we'll need to embed build metadata into the binaries.
Also, we're opting for SemVer-compatible CalVer here to align with the general product decision of using calendar versioning going forward.
Changes
versionoutput commands (lstk versionandlstk --version).goreleaser.yamlfor linux, macOS and Windows on ARM and AMDTests
Related
FLC-325