[ISSUE #1247]🔨Add Sync Issue Labels to PR Github action CI #1248
Conversation
WalkthroughA new GitHub Actions workflow named Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning Rate limit exceeded@mxsm has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 5 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. 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
|
|
🔊@mxsm 🚀Thanks for your contribution 🎉. CodeRabbit(AI) will review your code first 🔥 |
rocketmq-rust-bot
added
AI review first
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1248 +/- ##
=======================================
Coverage 18.26% 18.26%
=======================================
Files 428 428
Lines 53595 53595
=======================================
Hits 9789 9789
Misses 43806 43806 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
.github/workflows/sync-issue-labels.yml (2)
32-60: Implement rate limiting and batch processingThe current implementation processes issues sequentially without considering API rate limits. For PRs with many linked issues, this could hit rate limits.
Consider:
- Implementing exponential backoff for API calls
- Processing issues in batches
- Using GraphQL API instead of REST for better performance
Would you like me to provide an implementation example?
🧰 Tools
🪛 yamllint
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 58-58: trailing spaces
(trailing-spaces)
1-60: Fix formatting issuesThe file has several formatting issues:
- Incorrect newline characters
- Trailing spaces on multiple lines
Consider using a pre-commit hook with a YAML formatter to maintain consistent formatting.
🧰 Tools
🪛 yamllint
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 58-58: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/sync-issue-labels.yml(1 hunks)
🧰 Additional context used
🪛 yamllint
.github/workflows/sync-issue-labels.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 58-58: trailing spaces
(trailing-spaces)
| - name: Sync Labels from Linked Issue | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| github-token: ${{ secrets.BOT_TOKEN }} |
There was a problem hiding this comment.
Define minimum required permissions for BOT_TOKEN
Using secrets.BOT_TOKEN without explicit permissions could potentially grant more access than needed. Consider defining minimum required permissions in the workflow.
jobs:
sync-labels:
runs-on: ubuntu-latest
+ permissions:
+ issues: write
+ pull-requests: writeCommittable suggestion skipped: line range outside the PR's diff.
| issue_number: issueNumber, | ||
| }); | ||
|
|
||
| const issueLabels = issueResponse.data.labels.map(label => label.name); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle duplicate and invalid labels
The current implementation doesn't handle duplicate labels or validate if the labels exist in the repository.
- const issueLabels = issueResponse.data.labels.map(label => label.name);
+ // Remove duplicates and ensure labels exist
+ const issueLabels = [...new Set(issueResponse.data.labels.map(label => label.name))];
+
+ // Verify labels exist in the repository
+ const repoLabels = await github.rest.issues.listLabelsForRepo({
+ owner: context.repo.owner,
+ repo: context.repo.repo
+ });
+
+ const validLabels = issueLabels.filter(label =>
+ repoLabels.data.some(repoLabel => repoLabel.name === label)
+ );
+
+ if (validLabels.length !== issueLabels.length) {
+ console.log('Some labels were invalid or not found in the repository');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const issueLabels = issueResponse.data.labels.map(label => label.name); | |
| // Remove duplicates and ensure labels exist | |
| const issueLabels = [...new Set(issueResponse.data.labels.map(label => label.name))]; | |
| // Verify labels exist in the repository | |
| const repoLabels = await github.rest.issues.listLabelsForRepo({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo | |
| }); | |
| const validLabels = issueLabels.filter(label => | |
| repoLabels.data.some(repoLabel => repoLabel.name === label) | |
| ); | |
| if (validLabels.length !== issueLabels.length) { | |
| console.log('Some labels were invalid or not found in the repository'); | |
| } |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
🔊@mxsm 🚀Thanks for your contribution 🎉. CodeRabbit(AI) will review your code first 🔥 |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
🔊@mxsm 🚀Thanks for your contribution 🎉. CodeRabbit(AI) will review your code first 🔥 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
.github/workflows/sync-issue-labels.yml (3)
14-14: Consider using SHA pinning for third-party actionsWhile using version tags like
@v3and@v7is good, using the full SHA commit hash provides better security against supply chain attacks.- uses: actions/checkout@v3 + uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - uses: actions/github-script@v7 + uses: actions/github-script@d7906e4ad0b1822421a7e6a35d5ca353c962f410 # v7.0.1Also applies to: 18-18
52-57: Optimize label updates with batch processingInstead of adding labels one by one for each issue, consider collecting all unique labels first and then adding them in a single API call.
- await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number, - labels: issueLabels, - }); + // Collect all labels from all linked issues first + const allLabels = new Set(); + for (const label of issueLabels) { + allLabels.add(label); + } + + // Add all unique labels in a single API call at the end + if (allLabels.size > 0) { + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.payload.pull_request.number, + labels: Array.from(allLabels), + }); + }
1-60: Fix minor formatting issuesThe YAML file has some formatting inconsistencies:
- Trailing spaces on multiple lines
- Inconsistent line endings
Consider using an editor with YAML formatting support or adding a
.editorconfigfile to maintain consistent formatting.🧰 Tools
🪛 yamllint
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 58-58: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/sync-issue-labels.yml(1 hunks)
🧰 Additional context used
🪛 yamllint
.github/workflows/sync-issue-labels.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 58-58: trailing spaces
(trailing-spaces)
| const issueResponse = await github.rest.issues.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: issueNumber, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add rate limit handling for GitHub API calls
The GitHub API has rate limits that could affect the workflow. Consider adding rate limit detection and handling.
+ try {
+ const rateLimit = await github.rest.rateLimit.get();
+ if (rateLimit.data.rate.remaining < 10) {
+ console.log(`Rate limit critical: ${rateLimit.data.rate.remaining} calls remaining`);
+ await new Promise(resolve => setTimeout(resolve, 60000)); // Wait 1 minute
+ }
+
const issueResponse = await github.rest.issues.get({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNumber,
});
+ console.log(`Successfully fetched issue #${issueNumber}`);
+ } catch (error) {
+ if (error.status === 403 && error.message.includes('API rate limit exceeded')) {
+ console.log('Rate limit exceeded. Waiting before retry...');
+ await new Promise(resolve => setTimeout(resolve, 60000));
+ continue;
+ }
+ console.error(`Failed to fetch issue #${issueNumber}: ${error.message}`);
+ continue;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const issueResponse = await github.rest.issues.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: issueNumber, | |
| }); | |
| try { | |
| const rateLimit = await github.rest.rateLimit.get(); | |
| if (rateLimit.data.rate.remaining < 10) { | |
| console.log(`Rate limit critical: ${rateLimit.data.rate.remaining} calls remaining`); | |
| await new Promise(resolve => setTimeout(resolve, 60000)); // Wait 1 minute | |
| } | |
| const issueResponse = await github.rest.issues.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: issueNumber, | |
| }); | |
| console.log(`Successfully fetched issue #${issueNumber}`); | |
| } catch (error) { | |
| if (error.status === 403 && error.message.includes('API rate limit exceeded')) { | |
| console.log('Rate limit exceeded. Waiting before retry...'); | |
| await new Promise(resolve => setTimeout(resolve, 60000)); | |
| continue; | |
| } | |
| console.error(`Failed to fetch issue #${issueNumber}: ${error.message}`); | |
| continue; | |
| } |
rocketmq-rust-bot
added
approved
Which Issue(s) This PR Fixes(Closes)
Fixes #1247
Brief Description
How Did You Test This Change?
Summary by CodeRabbit