chore: improve build and test pipeline and the release ochestrator#529
Conversation
📝 WalkthroughWalkthroughThe PR updates two GitHub Actions workflows. Build-and-test adds pull-request-aware concurrency grouping and a pre-build check for commit images in GHCR to skip redundant builds. Release-orchestrator refactors from a single job into separate ChangesCI/CD Workflow Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Signed-off-by: Mevan <mevan.karu@gmail.com>
514c64f to
a367001
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/build-and-test.yml (1)
194-204: ⚡ Quick winReuse the SHA from the tag step to avoid duplication.
The SHA is calculated twice—once in the "Set image tag" step (line 186) and again here (line 197). This duplication violates DRY and could theoretically lead to inconsistency if the git state changes between steps.
♻️ Proposed fix to reuse the SHA output
- name: Check if image already exists id: check-image run: | - SHA=$(git rev-parse --short=8 HEAD) + SHA="${{ steps.tag.outputs.GIT_SHA_SHORT }}" if docker buildx imagetools inspect "${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${SHA}" >/dev/null 2>&1; then echo "Image ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${SHA} already exists. Skipping build." echo "skip=true" >> $GITHUB_OUTPUT else echo "Image not found. Proceeding with build." echo "skip=false" >> $GITHUB_OUTPUT fi🤖 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/build-and-test.yml around lines 194 - 204, The "Check if image already exists" step (id: check-image) recalculates the git SHA instead of reusing the SHA produced by the earlier "Set image tag" step; update the check-image step to reference the SHA output from the set-image-tag step (e.g. ${{ steps.set-image-tag.outputs.sha }}) when forming the image tag (instead of running git rev-parse again) so the build uses the single canonical SHA output and avoid duplication; ensure the referenced step id matches the actual id of the "Set image tag" step and keep using ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.set-image-tag.outputs.sha }} in the docker imagetools inspect and the messages..github/workflows/release-orchestrator.yml (1)
217-217: 💤 Low valueOptional: reduce
fetch-depthfor the branch and tag jobs.Both
branchandtagjobs only need the resolved SHA to create a local branch / annotated tag and push it — full history isn't required.fetch-depth: 1(the default) should suffice and avoids cloning the entire history on every release run. The same applies to thetagjob at line 257. Thevalidatejob still needsfetch-depth: 0forgit ls-remote/git rev-parsecorrectness.♻️ Proposed diff for both jobs
- name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: token: ${{ steps.app-token.outputs.token }} ref: ${{ needs.validate.outputs.checkout-sha }} - fetch-depth: 0 + fetch-depth: 1And in the
tagjob:- name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: token: ${{ steps.app-token.outputs.token }} ref: ${{ needs.validate.outputs.target-commit }} - fetch-depth: 0 + fetch-depth: 1🤖 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-orchestrator.yml at line 217, Update the checkout steps in the branch and tag jobs to use a shallower clone by changing fetch-depth from 0 to 1 (i.e., set fetch-depth: 1 on the actions/checkout step in the jobs named "branch" and "tag"); leave the validate job's fetch-depth at 0 so git ls-remote / git rev-parse behavior is preserved. Ensure both job definitions explicitly include the updated fetch-depth under the checkout step so only the needed commit SHA is fetched.
🤖 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.
Nitpick comments:
In @.github/workflows/build-and-test.yml:
- Around line 194-204: The "Check if image already exists" step (id:
check-image) recalculates the git SHA instead of reusing the SHA produced by the
earlier "Set image tag" step; update the check-image step to reference the SHA
output from the set-image-tag step (e.g. ${{ steps.set-image-tag.outputs.sha }})
when forming the image tag (instead of running git rev-parse again) so the build
uses the single canonical SHA output and avoid duplication; ensure the
referenced step id matches the actual id of the "Set image tag" step and keep
using ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{
steps.set-image-tag.outputs.sha }} in the docker imagetools inspect and the
messages.
In @.github/workflows/release-orchestrator.yml:
- Line 217: Update the checkout steps in the branch and tag jobs to use a
shallower clone by changing fetch-depth from 0 to 1 (i.e., set fetch-depth: 1 on
the actions/checkout step in the jobs named "branch" and "tag"); leave the
validate job's fetch-depth at 0 so git ls-remote / git rev-parse behavior is
preserved. Ensure both job definitions explicitly include the updated
fetch-depth under the checkout step so only the needed commit SHA is fetched.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1fee32e-617e-4eb5-8b29-8cbebb0695b2
📒 Files selected for processing (2)
.github/workflows/build-and-test.yml.github/workflows/release-orchestrator.yml
| run: | | ||
| SHA=$(git rev-parse --short=8 HEAD) | ||
| if docker buildx imagetools inspect "${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${SHA}" >/dev/null 2>&1; then | ||
| echo "Image ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${SHA} already exists. Skipping build." | ||
| echo "skip=true" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "Image not found. Proceeding with build." | ||
| echo "skip=false" >> $GITHUB_OUTPUT | ||
| fi |
There was a problem hiding this comment.
Suggested improvement:
run: |
SHA=$(git rev-parse --short=8 HEAD)
IMAGE="${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${SHA}"
echo "Checking if image exists: ${IMAGE}"
if docker buildx imagetools inspect "${IMAGE}" 2>&1; then
echo "Image ${IMAGE} already exists. Skipping build."
echo "skip=true" >> $GITHUB_OUTPUT
else
echo "Image not found. Proceeding with build."
echo "skip=false" >> $GITHUB_OUTPUT
fi
LakshanSS
left a comment
There was a problem hiding this comment.
Approved with a minor suggestion
$subject
Related to - openchoreo/openchoreo#3341
Summary by CodeRabbit