-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: rework tag and labels for Docker CI Build #222
Conversation
- Added the BUILDX_NO_DEFAULT_ATTESTATIONS environment variable to the GitHub Actions build workflow configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @psyray - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add documentation explaining why BUILDX_NO_DEFAULT_ATTESTATIONS is being disabled and what impact this has on the build process. This context is important for future maintainers.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Removed the BUILDX_NO_DEFAULT_ATTESTATIONS environment variable. - Replaced the manual versioning step with the docker/metadata-action to automate image tagging and labeling. - Consolidated platform specification directly in the buildx step, removing the matrix platform configuration.
- Modified the GitHub Actions workflow to update the conditions under which semantic versioning tags are enabled. - Introduced a new condition for enabling the latest tag when the reference is the master branch.
@sourcery-ai review |
Reviewer's Guide by SourceryThe PR updates the Docker CI build workflow by replacing manual version tagging with the docker/metadata-action and modifying how platform targets are specified. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @psyray - I've reviewed your changes - here's some feedback:
Overall Comments:
- The BUILDX_NO_DEFAULT_ATTESTATIONS environment variable mentioned in the description is not implemented in the workflow file. Please add it if it's intended to be part of this change.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This PR fix the Docker images CI build which not correctly set Arch leading to problem during install (manifest file not found).
Removal of the
platform
matrix since we'll handle both architectures in a single build stepAddition of the
Docker meta
step to manage tags and labels:rengine-${{ matrix.image }}-
for each imageModification of the
Build and push
step:platforms: linux/amd64,linux/arm64
This configuration should solve your issue because:
The resulting images will have tags like:
rengine-web-v2.1.0
rengine-web-latest
rengine-web-2.1
rengine-web-2
And each tag will properly support both architectures (amd64 and arm64).
This configuration will also allow:
On a
release/X.Y.Z
branch:latest
tagOn the master branch:
latest
tag will be generatedDuring a workflow_dispatch:
release/*
branch: version tags generation onlylatest
tag generationThus, the
latest
tag remains exclusive to master, while still allowing appropriate version tags to be generated from release branches.Summary by Sourcery
CI: