-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[ISSUE #1590]Add Label approved pull requests action #1591
Conversation
WalkthroughThis pull request introduces a new GitHub Actions workflow file named Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
🔊@rocketmq-rust-bot 🚀Thanks for your contribution 🎉. CodeRabbit(AI) will review your code first 🔥 |
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.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/label_approved_pull_requests.yml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/label_approved_pull_requests.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 17-17: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/label_approved_pull_requests.yml (2)
7-11
: LGTM!
The job configuration is appropriate for this use case.
14-19
:
Security and maintainability concerns in action configuration
Several issues need attention:
- Using
@master
tag is risky as it can break without warning - Single approval might not align with project's security requirements
- The action appears to be unmaintained (last updated 3 years ago)
Recommended changes:
- uses: pullreminders/label-when-approved-action@master
+ uses: pullreminders/label-when-approved-action@v1.0.7
env:
- APPROVALS: "1"
+ APPROVALS: "2"
- GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- #REMOVE_LABEL: ""
Let's verify the project's review requirements:
✅ Verification successful
Action configuration needs version pinning and cleanup
Based on the verification results:
- The repository has some protected branches but no strict review requirements
- Historical PRs show a consistent pattern of single approvals
- The repository uses auto-request-review workflow with custom configuration
Recommended changes:
- uses: pullreminders/label-when-approved-action@master
+ uses: pullreminders/label-when-approved-action@v1.0.7
env:
APPROVALS: "1" # Current repository practice
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- #REMOVE_LABEL: ""
- Pin the action version for stability
- Keep single approval as it aligns with current repository practices
- Remove commented out code
- Fix trailing whitespace
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check repository settings for required reviewers
gh api repos/:owner/:repo/branches/main/protection \
--jq '.required_pull_request_reviews.required_approving_review_count'
# Check recent merged PRs for typical approval count
gh pr list --state merged --limit 10 --json reviews \
--jq 'map({number: .number, approvals: [.reviews[].state] | map(select(. == "APPROVED")) | length})'
Length of output: 798
Script:
#!/bin/bash
# Check if there are any branch protection rules defined in the repository
gh api repos/:owner/:repo/branches --jq '.[].protected'
# Check if there are any CODEOWNERS files that might define review requirements
fd -g 'CODEOWNERS' -x cat {}
# Check if there are any other workflow files that might define review requirements
fd -g '*.yml' -g '*.yaml' -t f .github/workflows -x cat {} | grep -i "review"
Length of output: 1201
🧰 Tools
🪛 yamllint (1.35.1)
[error] 17-17: trailing spaces
(trailing-spaces)
name: Label approved pull requests | ||
|
||
on: | ||
pull_request_review: | ||
types: [submitted] | ||
|
||
jobs: | ||
labelWhenApproved: | ||
name: Label when approved | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- name: Label when approved | ||
uses: pullreminders/label-when-approved-action@master | ||
env: | ||
APPROVALS: "1" | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
ADD_LABEL: "approved" | ||
#REMOVE_LABEL: "" |
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.
Fix line endings
The file uses incorrect line ending characters. This can cause issues when the workflow runs on different operating systems.
Please ensure all line endings are normalized to LF (\n). You can fix this by:
- Adding a
.gitattributes
file:
+ *.yml text eol=lf
- Or running these git commands:
git config --global core.autocrlf input
git rm --cached -r .
git reset --hard
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 17-17: trailing spaces
(trailing-spaces)
on: | ||
pull_request_review: | ||
types: [submitted] |
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.
🛠️ Refactor suggestion
Consider additional trigger events for label consistency
The current trigger configuration might lead to stale labels in certain scenarios:
- When approvals are dismissed
- When PRs are updated after approval
Add these events to maintain label consistency:
on:
pull_request_review:
types: [submitted]
+ pull_request:
+ types: [synchronize]
Also consider using a more comprehensive action that handles these cases automatically, such as actions/label-approved-pulls@v1
which maintains label consistency across PR updates and approval dismissals.
Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1591 +/- ##
=======================================
Coverage 25.46% 25.46%
=======================================
Files 459 459
Lines 60352 60352
=======================================
Hits 15371 15371
Misses 44981 44981 ☔ View full report in Codecov by Sentry. |
Which Issue(s) This PR Fixes(Closes)
Fixes #1590
Brief Description
How Did You Test This Change?
Summary by CodeRabbit