feat: Version inject #34
Conversation
YaRissi
force-pushed
the
version-inject
branch
2 times, most recently
from
5cb7580 to
d78b7eb
Compare
YaRissi
force-pushed
the
version-inject
branch
from
d78b7eb to
94e237e
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughRefactors version handling to an accessor, adds a CI Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Release trigger
participant Build as Goreleaser
participant CI as release-cli job
participant Updater as update-version job
participant Repo as Repository
participant Git as Git Remote
Dev->>Build: create GitHub Release (tag)
Build->>CI: run release-cli pipeline
CI->>Build: produce artifacts (ldflags set internal/version.version)
Build-->>Repo: artifacts published
CI->>Updater: trigger update-version job
Updater->>Repo: checkout branch `v4.x`
Updater->>Updater: parse tag from GITHUB_REF
Updater->>Repo: modify `internal/version/checker.go` version var
Updater->>Git: commit "chore: bump version to $TAG"
Updater->>Git: push branch `v4.x`
Note right of Build: GetVersion() returns injected `internal/version.version`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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: 1
🧹 Nitpick comments (2)
.github/workflows/release-cli.yml (1)
39-48: Add error handling and validation to the version update workflow.The workflow lacks safeguards for several failure scenarios:
- No verification that the sed command actually modified the file
- No validation that TAG was extracted correctly
- Push failures are not explicitly handled
Consider adding validation and error handling:
- name: Update version file run: | TAG=${GITHUB_REF#refs/tags/} + if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+ ]]; then + echo "Error: Invalid tag format: $TAG" + exit 1 + fi echo "Updating version to $TAG" sed -i "s/^\tversion = \".*\"/\tversion = \"$TAG\"/" internal/version/checker.go + if ! git diff --quiet internal/version/checker.go; then + echo "Version file updated successfully" + else + echo "Warning: No changes detected in version file" + exit 1 + fi git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git add internal/version/checker.go git commit -m "chore: bump version to $TAG" git push origin v4.xinternal/version/checker.go (1)
16-23: Consider using "dev" or "unknown" as the default version.The default version
"v1.0.3"could be misleading if the binary is built without proper ldflags injection (e.g., during local development withgo build). Users might incorrectly believe they're running v1.0.3.Consider using a more obvious development version:
// Version variables injected by GoReleaser at build time via ldflags var ( - version = "v1.0.3" + version = "dev" )Alternatively,
"unknown"or"v0.0.0-dev"would also clearly indicate an unofficial build.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/release-cli.yml(1 hunks).goreleaser.yml(2 hunks)HOW-TO-RELEASE.md(4 hunks)cmd/root.go(2 hunks)cmd/update/update.go(1 hunks)cmd/version/version.go(1 hunks)internal/version/checker.go(3 hunks)
🔇 Additional comments (9)
cmd/version/version.go (1)
16-16: LGTM!The migration from
version.CliVersiontoversion.GetVersion()is correct and consistent with the refactoring to support build-time version injection.cmd/update/update.go (1)
30-30: LGTM!The migration to
version.GetVersion()is correct and maintains the existing update comparison logic.cmd/root.go (2)
71-71: LGTM!The version string formatting correctly uses
version.GetVersion()to display the CLI version in the help text.
156-156: LGTM!The update check logic correctly uses
version.GetVersion()for version comparison..goreleaser.yml (1)
6-7: LGTM!The ldflags configuration correctly targets
github.com/coollabsio/coolify-cli/internal/version.versionto inject the version at build time. This aligns with the privateversionvariable ininternal/version/checker.go.HOW-TO-RELEASE.md (1)
47-48: LGTM!Documentation correctly reflects the new version injection approach and the rename from
CliVersiontoGetVersion().Also applies to: 114-114
internal/version/checker.go (2)
21-23: LGTM!The
GetVersion()accessor function provides a clean interface for retrieving the build-time injected version, and the unexportedversionvariable correctly follows Go conventions for variables modified via ldflags.
42-42: LGTM!The internal usages correctly call
GetVersion()instead of directly accessing the oldCliVersionconstant.Also applies to: 96-96
.github/workflows/release-cli.yml (1)
29-48: Unable to verify branch protection settings due to insufficient API permissions.The GitHub API returned a 403 Forbidden error when attempting to query branch protection rules for the
v4.xbranch. This indicates the current authentication context lacks permissions to view these settings—not that protection rules don't exist.Manually verify in repository settings whether the
v4.xbranch has protection rules enabled and confirm the GitHub Actions bot is allowed to bypass them if needed.
|
@YaRissi can you please check if the rabbit makes sense? :) |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@andrasbacsai fixed all issues |
|
Oh, I merged the other PR's and it caused some conflicts. Sorry. Can you please fix them? |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/release-cli.yml (1)
44-46: Validate the extracted TAG before using it.The TAG is extracted from GITHUB_REF without validation. If GITHUB_REF has an unexpected format or the tag is empty, the sed command could produce incorrect results or the commit message could be malformed.
Apply this diff to add validation:
TAG=${GITHUB_REF#refs/tags/} + if [[ -z "$TAG" ]] || [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.*)?$ ]]; then + echo "Error: Invalid or missing tag format. Expected vX.Y.Z, got: $TAG" + exit 1 + fi echo "Updating version to $TAG" sed -i "s/^\tversion = \".*\"/\tversion = \"$TAG\"/" internal/version/checker.gointernal/version/checker.go (1)
17-20: Consider updating the comment to singular form.The comment mentions "Version variables" (plural), but only one variable is declared. While this is minor, it could be clearer.
Apply this diff:
-// Version variables injected by GoReleaser at build time via ldflags +// version is injected by GoReleaser at build time via ldflags. +// The default value serves as a fallback for `go install` builds. var ( version = "v1.0.3" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/release-cli.yml(1 hunks).goreleaser.yml(1 hunks)HOW-TO-RELEASE.md(4 hunks)cmd/root.go(2 hunks)cmd/update/update.go(1 hunks)cmd/version/version.go(1 hunks)internal/version/checker.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/root.go
- cmd/update/update.go
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/version/version.go (1)
internal/version/checker.go (1)
GetVersion(22-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (4)
.goreleaser.yml (1)
15-15: LGTM! Version injection is correctly configured.The ldflags correctly targets the unexported
versionvariable ininternal/version/checker.goand uses the standard Goreleaser{{ .Version }}template. This will properly inject the release tag into binaries at build time.cmd/version/version.go (1)
17-17: LGTM! Correctly uses the new version accessor.The change from
version.CliVersiontoversion.GetVersion()is correct and consistent with the refactoring across the codebase. This enables build-time version injection via Goreleaser.internal/version/checker.go (2)
22-24: LGTM! Clean accessor implementation.The
GetVersion()function provides a clean, thread-safe way to access the build-time injected version. This pattern correctly enables ldflags injection, which wouldn't work with a constant or direct variable export.
43-43: LGTM! GetVersion() calls are correctly placed.Both usages of
GetVersion()are appropriate:
- Line 43: Returns the current version when skipping the update check
- Line 100: Retrieves the current version for semantic version comparison
The refactoring from
CliVersiontoGetVersion()is complete and consistent throughout the file.Also applies to: 100-100
|
@andrasbacsai fixed the merge conflicts |
|
Thank you for the PR! 💜 |
2 participants
Changes
Why
Simplify the release process, less manual steps
Summary by CodeRabbit
Chores
Documentation
User-facing