build: use PROJECT_VERSION as single source of truth#27
Conversation
Replace git describe --tags with PROJECT_VERSION from CMakeLists.txt. Git short SHA appended as build metadata (e.g. 1.9.0.2-1b09ac55).
Trigger on CMakeLists.txt changes to master instead of tag push. Extract version from PROJECT_VERSION, skip build if image exists.
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe release process is reworked to derive the version from CMakeLists.txt instead of git tags/describe, introducing a separate GIT_SHORT_SHA build argument. The release workflow now triggers on master pushes touching CMakeLists.txt, checks for existing image tags before rebuilding, and Dockerfiles/Makefile propagate GIT_SHORT_SHA instead of OLR_VERSION. ChangesVersion and build-arg rework
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant CMakeLists as CMakeLists.txt
participant Registry as ghcr.io
GitHub->>CMakeLists: extract VERSION
GitHub->>Registry: docker manifest inspect image:VERSION
alt tag exists
Registry-->>GitHub: manifest found
GitHub->>GitHub: skip build/push
else tag missing
Registry-->>GitHub: manifest not found
GitHub->>GitHub: buildx setup + login
GitHub->>Registry: build and push image:VERSION with GIT_SHORT_SHA
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile.release (1)
8-14: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUpdate stale usage example — still references removed
OLR_VERSIONbuild-arg.The header comment still shows
OLR_VERSION=$(git describe --tags --always --dirty)and--build-arg OLR_VERSION=$OLR_VERSION, but the builder stage now takesGIT_SHORT_SHA(line 26) instead. Following this example verbatim would silently no-op the version override.📝 Proposed fix
-# OLR_VERSION=$(git describe --tags --always --dirty) +# GIT_SHORT_SHA=$(git rev-parse --short HEAD) # docker buildx build -f Dockerfile.release \ -# --build-arg OLR_VERSION=$OLR_VERSION \ +# --build-arg GIT_SHORT_SHA=$GIT_SHORT_SHA \ # --build-arg GIDOLR=$(id -g) --build-arg UIDOLR=$(id -u) \ # --build-arg WITHORACLE=1 --build-arg WITHKAFKA=1 \ # --build-arg WITHPROTOBUF=1 --build-arg WITHPROMETHEUS=1 \ -# -t rophy/openlogreplicator:$OLR_VERSION . +# -t rophy/openlogreplicator:$GIT_SHORT_SHA .🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile.release` around lines 8 - 14, The usage example in Dockerfile.release is stale because it still documents the removed OLR_VERSION build arg. Update the example to match the current builder inputs by referencing GIT_SHORT_SHA instead, and make sure the sample build command uses the same version source expected by the Dockerfile so readers do not follow an argument that is ignored. Use the existing Dockerfile.release build-arg example block as the location to revise.
🧹 Nitpick comments (2)
.github/workflows/release.yaml (1)
27-35: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPrefer
env:binding over inline template expansion inrun:scripts.Static analysis flags potential template-injection risk from interpolating
${{ steps.version.outputs.version }}directly into the shell script. Even though the value here is derived from trusted repo content, the safer idiom is to bind it to an environment variable and reference it as a shell variable.🛡️ Proposed fix
- name: Check if image already exists id: check run: | - if docker manifest inspect "${{ env.IMAGE }}:${{ steps.version.outputs.version }}" > /dev/null 2>&1; then + if docker manifest inspect "${IMAGE}:${VERSION}" > /dev/null 2>&1; then echo "exists=true" >> "$GITHUB_OUTPUT" - echo "Image ${{ env.IMAGE }}:${{ steps.version.outputs.version }} already exists, skipping build" + echo "Image ${IMAGE}:${VERSION} already exists, skipping build" else echo "exists=false" >> "$GITHUB_OUTPUT" fi env: DOCKER_CLI_EXPERIMENTAL: enabled + VERSION: ${{ steps.version.outputs.version }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yaml around lines 27 - 35, Bind the image version used in the “Check if image already exists” step to an env variable instead of interpolating steps.version.outputs.version directly inside the run script. Update the step that uses docker manifest inspect and the echo output to reference a shell variable populated from env:, so the shell script no longer contains inline template expansion. Keep the existing behavior and output, just move the version value into the step’s environment.Source: Linters/SAST tools
Makefile (1)
38-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated version-extraction regex between Makefile and release.yaml.
The
grep -oP 'VERSION \K[0-9]+(\.[0-9]+)+' CMakeLists.txt | head -1logic is duplicated verbatim in.github/workflows/release.yaml(line 23). If the CMakeLists.txt format changes, both call sites must be updated in lockstep, or the two version sources will drift.Consider extracting into a small shared script (e.g.
scripts/get_version.sh) invoked from both places.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` at line 38, The version-extraction logic is duplicated between the Makefile and the release workflow, so update the Makefile’s image tag generation to delegate to a shared helper instead of embedding the regex directly. Add a small reusable version script (for example, a script invoked by the Makefile target and by the release workflow) and have both call that shared entry point so the CMakeLists.txt parsing lives in one place. Reference the existing Makefile tag rule and the release workflow’s version lookup so the implementation stays consistent across both call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yaml:
- Around line 50-59: The release workflow is passing the full commit hash into
GIT_SHORT_SHA, which breaks the intended short-SHA version format. Update the
Build and push release image step in the release workflow to use a truncated SHA
instead of github.sha directly, preferably by adding a prior short_sha step and
wiring its output into the build-args. Make sure the value passed to Docker
matches the GIT_SHORT_SHA contract used by CMakeLists.txt and the version
tagging logic.
---
Outside diff comments:
In `@Dockerfile.release`:
- Around line 8-14: The usage example in Dockerfile.release is stale because it
still documents the removed OLR_VERSION build arg. Update the example to match
the current builder inputs by referencing GIT_SHORT_SHA instead, and make sure
the sample build command uses the same version source expected by the Dockerfile
so readers do not follow an argument that is ignored. Use the existing
Dockerfile.release build-arg example block as the location to revise.
---
Nitpick comments:
In @.github/workflows/release.yaml:
- Around line 27-35: Bind the image version used in the “Check if image already
exists” step to an env variable instead of interpolating
steps.version.outputs.version directly inside the run script. Update the step
that uses docker manifest inspect and the echo output to reference a shell
variable populated from env:, so the shell script no longer contains inline
template expansion. Keep the existing behavior and output, just move the version
value into the step’s environment.
In `@Makefile`:
- Line 38: The version-extraction logic is duplicated between the Makefile and
the release workflow, so update the Makefile’s image tag generation to delegate
to a shared helper instead of embedding the regex directly. Add a small reusable
version script (for example, a script invoked by the Makefile target and by the
release workflow) and have both call that shared entry point so the
CMakeLists.txt parsing lives in one place. Reference the existing Makefile tag
rule and the release workflow’s version lookup so the implementation stays
consistent across both call sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 41fcb6ce-0c60-408a-ba34-399e803d568a
📒 Files selected for processing (5)
.github/workflows/release.yamlCMakeLists.txtDockerfile.devDockerfile.releaseMakefile
- Truncate github.sha to 8 chars for GIT_SHORT_SHA - Use env binding instead of inline template in shell scripts - Update stale usage comment in Dockerfile.release
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
git describe --tagswithPROJECT_VERSIONfrom CMakeLists.txt as the version source1.9.0.2-1b09ac55)1.9.0.2Files changed
CMakeLists.txt— version bump + git SHA logicMakefile— passGIT_SHORT_SHAinstead ofOLR_VERSIONDockerfile.dev/Dockerfile.release—ARG GIT_SHORT_SHAreplacesARG OLR_VERSION.github/workflows/release.yaml— trigger on cmake change, extract version, skip if existsTest plan
make build→ version shows1.9.0.2-1b09ac55Summary by CodeRabbit
New Features
Bug Fixes