add release-image.yml - #39
Merged
Merged
Conversation
Reviewer's GuideIntroduces two new GitHub workflows to automate building and pushing Docker images for the API and web components across AMD64 and ARM64 architectures, including registry authentication, QEMU/buildx setup, caching, and concurrency control. Flow diagram for Docker image build and push process (API & Web)flowchart TD
Start([Start Workflow])
Checkout[Checkout Code]
Login[Login to Aliyun Registry]
QEMU[Set up QEMU]
Buildx[Set up Docker Buildx]
BuildAMD[Build & Push AMD64 Image]
BuildARM[Build & Push ARM64 Image]
End([End])
Start --> Checkout --> Login --> QEMU --> Buildx
Buildx --> BuildAMD --> BuildARM --> End
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @konpure - I've reviewed your changes - here's some feedback:
- Consider refactoring duplicated steps in release-image-api.yml and release-image-web.yml into a reusable workflow or using a matrix to reduce duplication.
- The
context: "{{defaultContext}}:api"syntax looks off—use the actual path (e.g../api) or${{ github.workspace }}/apifor the build context. - Instead of pushing separate arch-specific tags, use a single multi-platform buildx invocation to publish a manifest list so docker pull automatically selects the correct architecture.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider refactoring duplicated steps in release-image-api.yml and release-image-web.yml into a reusable workflow or using a matrix to reduce duplication.
- The `context: "{{defaultContext}}:api"` syntax looks off—use the actual path (e.g. `./api`) or `${{ github.workspace }}/api` for the build context.
- Instead of pushing separate arch-specific tags, use a single multi-platform buildx invocation to publish a manifest list so docker pull automatically selects the correct architecture.
## Individual Comments
### Comment 1
<location> `.github/workflows/release-image-api.yml:11` </location>
<code_context>
+ required: true
+ push:
+ branches: [release-ci]
+ pull_request:
+ types: [closed]
+ branches: [release-ci]
+
+concurrency:
</code_context>
<issue_to_address>
Workflow triggers on pull_request closed may cause unnecessary builds.
To avoid unnecessary builds when a PR is closed without merging, add a condition to check if 'github.event.pull_request.merged == true' in your workflow job.
Suggested implementation:
```
jobs:
# Only run jobs if the PR was merged (not just closed)
# Add this 'if' condition to each job definition below, for example:
# build:
# if: github.event.pull_request.merged == true
```
You must add `if: github.event.pull_request.merged == true` to the top level of each job definition in this workflow file.
For example, if you have a job called `build:`, add the line directly under the job name:
build:
if: github.event.pull_request.merged == true
runs-on: ubuntu-latest
...
Repeat this for every job that should only run when the PR is merged.
</issue_to_address>
### Comment 2
<location> `.github/workflows/release-image-web.yml:11` </location>
<code_context>
+ required: true
+ push:
+ branches: [release-ci]
+ pull_request:
+ types: [closed]
+ branches: [release-ci]
+
+concurrency:
</code_context>
<issue_to_address>
Workflow triggers on pull_request closed may cause unnecessary builds.
Consider refining the trigger or adding a condition so builds only run when PRs are merged, not just closed.
Suggested implementation:
```
jobs:
build-and-push:
if: github.event_name != 'pull_request' || github.event.pull_request.merged == true
```
- If you already have a job defined (e.g., `build-and-push`), add the `if:` condition to that job instead of creating a new job block as above.
- If there are multiple jobs, apply the `if:` condition to each job that should only run when the PR is merged.
- Remove the extra indentation if you are not introducing a new job block.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| description: 'The tag to release' | ||
| required: true | ||
| push: | ||
| branches: [release-ci] |
There was a problem hiding this comment.
suggestion: Workflow triggers on pull_request closed may cause unnecessary builds.
To avoid unnecessary builds when a PR is closed without merging, add a condition to check if 'github.event.pull_request.merged == true' in your workflow job.
Suggested implementation:
jobs:
# Only run jobs if the PR was merged (not just closed)
# Add this 'if' condition to each job definition below, for example:
# build:
# if: github.event.pull_request.merged == true
You must add if: github.event.pull_request.merged == true to the top level of each job definition in this workflow file.
For example, if you have a job called build:, add the line directly under the job name:
build:
if: github.event.pull_request.merged == true
runs-on: ubuntu-latest
...
Repeat this for every job that should only run when the PR is merged.
| description: 'The tag to release' | ||
| required: true | ||
| push: | ||
| branches: [release-ci] |
There was a problem hiding this comment.
suggestion: Workflow triggers on pull_request closed may cause unnecessary builds.
Consider refining the trigger or adding a condition so builds only run when PRs are merged, not just closed.
Suggested implementation:
jobs:
build-and-push:
if: github.event_name != 'pull_request' || github.event.pull_request.merged == true
- If you already have a job defined (e.g.,
build-and-push), add theif:condition to that job instead of creating a new job block as above. - If there are multiple jobs, apply the
if:condition to each job that should only run when the PR is merged. - Remove the extra indentation if you are not introducing a new job block.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Tip
Close issue syntax:
Fixes #<issue number>orResolves #<issue number>, see documentation for more details.Screenshots
Checklist
Important
Please review the checklist below before submitting your pull request.
dev/reformat(backend) andcd web && npx lint-staged(frontend) to appease the lint godsSummary by Sourcery
Add GitHub Actions workflows to automate building and pushing multi-architecture Docker images for both API and Web components.
New Features:
CI: