feat: add docker latest update ci#755
Conversation
📝 WalkthroughWalkthroughUpdated the Docker release workflow to support beta release semantics by introducing an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/docker-release.yml (1)
68-72: Clarifypretag semantics in documentation.The
pretag is always updated for both beta and formal releases, which might be counterintuitive since "pre" typically suggests pre-release content. Based on the header comments, this appears intentional (thepretag serves as a "latest tested" pointer), but consider adding a brief comment explaining its purpose to avoid confusion for future maintainers.📝 Suggested documentation improvement
# Build CoPaw multi-arch Docker image and push to DockerHub + Aliyun ACR on release. # Beta release: update <version> and pre only. # Formal release: update <version>, pre and latest. +# Note: 'pre' tag always points to the most recent build (beta or formal). +# 'latest' tag only points to formal/stable releases. name: Docker Build and Push on Release🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-release.yml around lines 68 - 72, Add a brief inline comment above the TAGS construction (the TAGS variable block that appends "-t ...:pre" and the conditional that checks IS_BETA) explaining that the "pre" tag is intentionally always updated for both beta and formal releases and serves as a "latest tested" pointer rather than a traditional pre-release tag; mention that IS_BETA still controls the "latest" tag but does not affect the "pre" tag to avoid future confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/docker-release.yml:
- Around line 68-72: Add a brief inline comment above the TAGS construction (the
TAGS variable block that appends "-t ...:pre" and the conditional that checks
IS_BETA) explaining that the "pre" tag is intentionally always updated for both
beta and formal releases and serves as a "latest tested" pointer rather than a
traditional pre-release tag; mention that IS_BETA still controls the "latest"
tag but does not affect the "pre" tag to avoid future confusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 696d8d09-537c-4706-ae5f-b50d4fba84f3
📒 Files selected for processing (1)
.github/workflows/docker-release.yml
There was a problem hiding this comment.
Pull request overview
Adds support for conditionally pushing a latest Docker tag only on formal (non-beta) releases.
Changes:
- Added a
workflow_dispatchboolean input (is_beta) to control whetherlatestis updated. - Added a step to normalize
versionandis_betavalues from eitherreleaseorworkflow_dispatchevents. - Updated tagging logic to include
latestonly whenis_betais nottrue.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TAGS="-t ${ACR_REGISTRY}/${IMAGE}:${VERSION} -t ${ACR_REGISTRY}/${IMAGE}:pre" | ||
| TAGS="${TAGS} -t docker.io/${IMAGE}:${VERSION} -t docker.io/${IMAGE}:pre" | ||
| if [ "${IS_BETA}" != "true" ]; then | ||
| TAGS="${TAGS} -t ${ACR_REGISTRY}/${IMAGE}:latest -t docker.io/${IMAGE}:latest" | ||
| fi |
There was a problem hiding this comment.
Building up TAGS as a single string is brittle because it relies on later word-splitting to turn it into arguments (and can misbehave if any variable ever contains whitespace or unexpected characters). Prefer using a Bash array for tags and pass it as separate arguments (e.g., TAGS=( -t ... -t ... ) then use \"${TAGS[@]}\" when invoking docker buildx build).
| if [ -n "${{ github.event.release.tag_name }}" ]; then | ||
| echo "version=${{ github.event.release.tag_name }}" >> $GITHUB_OUTPUT | ||
| echo "is_beta=${{ github.event.release.prerelease }}" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "version=${{ github.event.inputs.version }}" >> $GITHUB_OUTPUT | ||
| echo "is_beta=${{ github.event.inputs.is_beta }}" >> $GITHUB_OUTPUT | ||
| fi |
There was a problem hiding this comment.
The event detection logic is currently based on whether github.event.release.tag_name is non-empty. It’s clearer and more robust to branch on the actual event type (e.g., github.event_name == 'release') so the intent is explicit and won’t be affected by payload shape changes.
| echo "version=${{ github.event.release.tag_name }}" >> $GITHUB_OUTPUT | ||
| echo "is_beta=${{ github.event.release.prerelease }}" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "version=${{ github.event.inputs.version }}" >> $GITHUB_OUTPUT | ||
| echo "is_beta=${{ github.event.inputs.is_beta }}" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
$GITHUB_OUTPUT should be quoted when redirecting (i.e., >> \"$GITHUB_OUTPUT\") to avoid edge cases with unusual paths. Using printf instead of echo is also more predictable for output formatting.
Description
add docker latest update ci
Type of Change
Component(s) Affected
Checklist
pre-commit run --all-fileslocally and it passespytestor as relevant) and they passTesting
[How to test these changes]
Local Verification Evidence
Additional Notes
[Optional: any other context]
Summary by CodeRabbit