[ISSUE #4291]♻️Refactor CI workflows to separate auto-approve and auto-merge actions for improved clarity and maintainability#4293
[ISSUE #4291]♻️Refactor CI workflows to separate auto-approve and auto-merge actions for improved clarity and maintainability#4293
Conversation
…o-merge actions for improved clarity and maintainability
…o-merge actions for improved clarity and maintainability
…o-merge actions for improved clarity and maintainability
rocketmq-rust-bot
requested review from
SpaceXCN,
TeslaRustor and
rocketmq-rust-bot
|
🔊@mxsm 🚀Thanks for your contribution🎉！ 💡CodeRabbit(AI) will review your code first🔥！ Note 🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥. |
rocketmq-rust-bot
added
AI review first
|
Warning Rate limit exceeded@mxsm has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
✨ 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the CI/CD workflow by extracting the auto-approve and auto-merge functionality from the main CI workflow into separate, dedicated workflow files for improved modularity and maintainability.
- Removes auto-approve and auto-merge jobs from
ci.yaml - Creates standalone
auto-approve.ymlworkflow that waits for CI checks and auto-approves PRs with the "auto merge" label - Creates standalone
auto-merge.ymlworkflow that handles automatic merging after approval
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
.github/workflows/ci.yaml |
Removed the auto-approve and auto-merge jobs to simplify the main CI workflow |
.github/workflows/auto-approve.yml |
New workflow that auto-approves PRs after CI checks pass |
.github/workflows/auto-merge.yml |
New workflow that handles automatic merging of approved PRs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ref: ${{ github.event.pull_request.head.sha }} | ||
| repo: ${{ github.repository }} | ||
| wait-interval: 20 | ||
| running-workflow-name: "Rocketmq Rust CI" |
There was a problem hiding this comment.
The running-workflow-name parameter should match the exact name of the CI workflow, which is "Rocketmq Rust CI". However, the wait-on-check-action expects check names, not workflow names. For GitHub Actions workflows, the check names are typically the job names (e.g., "Check (fmt + clippy)", "Build & Test (ubuntu-latest)", "Code Coverage"), not the workflow name.
Consider either:
- Waiting for specific job names instead of the workflow name
- Using a different approach to ensure CI has completed
Alternatively, you may need to verify if the action correctly handles workflow names in the context of GitHub Actions.
| running-workflow-name: "Rocketmq Rust CI" | |
| check-name: "Check (fmt + clippy), Build & Test (ubuntu-latest), Code Coverage" |
|
|
||
| steps: | ||
| - name: Wait for Rocketmq Rust CI to succeed | ||
| uses: lewagon/wait-on-check-action@v1.1.0 |
There was a problem hiding this comment.
[nitpick] The version v1.1.0 of lewagon/wait-on-check-action may be outdated. Consider using a more recent version (e.g., v1.3.4 or later) which includes bug fixes and improvements. Check the action's repository for the latest stable version.
| uses: lewagon/wait-on-check-action@v1.1.0 | |
| uses: lewagon/wait-on-check-action@v1.3.4 |
| with: | ||
| github-token: ${{ secrets.BOT_TOKEN }} | ||
| pull-request-number: ${{ github.event.pull_request.number }} | ||
| review-message: "LGTM✅ All required checks passed. Auto-approved for merge." No newline at end of file |
There was a problem hiding this comment.
Missing space between "LGTM" and the emoji. Should be "LGTM ✅" for better readability.
| review-message: "LGTM✅ All required checks passed. Auto-approved for merge." | |
| review-message: "LGTM ✅ All required checks passed. Auto-approved for merge." |
| types: [ completed ] | ||
|
|
||
| permissions: | ||
| pull-requests: read |
There was a problem hiding this comment.
The auto-approve action requires pull-requests: write permission to approve pull requests, but the current permissions only grant pull-requests: read. This will cause the approval step to fail with a permission error.
Add pull-requests: write to the permissions section:
permissions:
pull-requests: write
contents: read
checks: read| pull-requests: read | |
| pull-requests: write |
| check_suite: | ||
| types: [ completed ] |
There was a problem hiding this comment.
When this workflow is triggered by a check_suite event (line 6-7), the github.event.pull_request context used in the condition (lines 18-19) and in the steps (lines 26, 35) will not be available, causing the workflow to fail or skip unexpectedly.
Consider removing the check_suite trigger or adding logic to handle this event type differently. The workflow will already be triggered by pull_request_target events when the PR is updated or labeled.
| check_suite: | |
| types: [ completed ] |
rocketmq-rust-bot
added
approved
Which Issue(s) This PR Fixes(Closes)
Fixes #4291
Brief Description
How Did You Test This Change?