-
-
Notifications
You must be signed in to change notification settings - Fork 712
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
feat: Workflow changes which will label issues automatically as per #2256 #2390
feat: Workflow changes which will label issues automatically as per #2256 #2390
Conversation
WalkthroughThe changes introduce a new JSON5 configuration file for GitHub workflows that defines label synonyms and default labels. Modifications to the existing workflow file enhance the automation of issue labeling based on specific keywords in issue descriptions. The updated workflow now checks for keywords related to dependencies, security, and design, applying appropriate labels accordingly. The configuration aims to streamline the labeling process, reducing manual overhead. Changes
Assessment against linked issues
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
|
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
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.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
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: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/auto-label.json5
(1 hunks).github/workflows/issue.yml
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/auto-label.json5
🧰 Additional context used
🪛 yamllint
.github/workflows/issue.yml
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 54-54: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/issue.yml (2)
21-25
: LGTM! Efficient repository checkout configuration.
The sparse checkout configuration is well-optimized, only fetching the required configuration file.
26-28
: Verify auto-label configuration file existence and format.
The workflow depends on .github/workflows/auto-label.json5
, but we should verify its existence and format.
✅ Verification successful
Configuration file exists and is properly formatted
The auto-label configuration file .github/workflows/auto-label.json5
exists and contains valid JSON5 with the expected structure:
- Defines label synonyms for dependencies, security, and ui/ux categories
- Includes defaultLabels configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the config file exists and is valid JSON5
if [ -f ".github/workflows/auto-label.json5" ]; then
echo "Config file exists"
# Use grep to check for expected content structure
rg -U "labelsSynonyms.*dependencies.*security.*ui/ux" .github/workflows/auto-label.json5
else
echo "Error: auto-label.json5 not found"
exit 1
fi
Length of output: 185
Script:
#!/bin/bash
# Let's check if the file exists and show its contents
if [ -f ".github/workflows/auto-label.json5" ]; then
echo "=== Content of auto-label.json5 ==="
cat .github/workflows/auto-label.json5
else
echo "File not found: .github/workflows/auto-label.json5"
fi
# Also check for any similar config files in case it's in a different location
echo -e "\n=== Similar config files ==="
fd -g "*auto-label*.json*"
Length of output: 492
.github/workflows/issue.yml
Outdated
if(labels.data.reduce((a, c)=>a||["dependencies"].includes(c.name), false)) | ||
await github.rest.issues.addLabels({ | ||
owner: context.repo.owner, | ||
repo: context.repo.repo, | ||
issue_number: context.issue.number, | ||
labels: ["good first issue", "security"] | ||
}); | ||
else if(labels.data.reduce((a, c)=>a||["security", "ui/ux"].includes(c.name), false)) | ||
await github.rest.issues.addLabels({ | ||
owner: context.repo.owner, | ||
repo: context.repo.repo, | ||
issue_number: context.issue.number, | ||
labels: ["good first issue"] | ||
}); |
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.
Logic doesn't match requirements - should check issue body for keywords.
The current implementation checks existing labels instead of scanning the issue body for keywords as specified in the requirements. Additionally, some required label combinations are missing.
Here's the corrected implementation:
- if(labels.data.reduce((a, c)=>a||["dependencies"].includes(c.name), false))
- await github.rest.issues.addLabels({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: context.issue.number,
- labels: ["good first issue", "security"]
- });
- else if(labels.data.reduce((a, c)=>a||["security", "ui/ux"].includes(c.name), false))
- await github.rest.issues.addLabels({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: context.issue.number,
- labels: ["good first issue"]
- });
+ const issue = await github.rest.issues.get({
+ owner,
+ repo,
+ issue_number
+ });
+
+ const body = issue.data.body.toLowerCase();
+ const labelsToAdd = new Set(["good first issue"]);
+
+ if (body.match(/\b(dependabot|dependency|dependencies)\b/)) {
+ labelsToAdd.add("dependencies").add("security");
+ }
+ if (body.match(/\bsecurity\b/)) {
+ labelsToAdd.add("security");
+ }
+ if (body.match(/\b(layout|screen|design|figma)\b/)) {
+ labelsToAdd.add("ui/ux");
+ }
+
+ if (labelsToAdd.size > 1) { // More than just "good first issue"
+ await github.rest.issues.addLabels({
+ owner,
+ repo,
+ issue_number,
+ labels: Array.from(labelsToAdd)
+ });
+ }
📝 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.
if(labels.data.reduce((a, c)=>a||["dependencies"].includes(c.name), false)) | |
await github.rest.issues.addLabels({ | |
owner: context.repo.owner, | |
repo: context.repo.repo, | |
issue_number: context.issue.number, | |
labels: ["good first issue", "security"] | |
}); | |
else if(labels.data.reduce((a, c)=>a||["security", "ui/ux"].includes(c.name), false)) | |
await github.rest.issues.addLabels({ | |
owner: context.repo.owner, | |
repo: context.repo.repo, | |
issue_number: context.issue.number, | |
labels: ["good first issue"] | |
}); | |
const issue = await github.rest.issues.get({ | |
owner, | |
repo, | |
issue_number | |
}); | |
const body = issue.data.body.toLowerCase(); | |
const labelsToAdd = new Set(["good first issue"]); | |
if (body.match(/\b(dependabot|dependency|dependencies)\b/)) { | |
labelsToAdd.add("dependencies").add("security"); | |
} | |
if (body.match(/\bsecurity\b/)) { | |
labelsToAdd.add("security"); | |
} | |
if (body.match(/\b(layout|screen|design|figma)\b/)) { | |
labelsToAdd.add("ui/ux"); | |
} | |
if (labelsToAdd.size > 1) { // More than just "good first issue" | |
await github.rest.issues.addLabels({ | |
owner, | |
repo, | |
issue_number, | |
labels: Array.from(labelsToAdd) | |
}); | |
} |
🧰 Tools
🪛 yamllint
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 54-54: trailing spaces
(trailing-spaces)
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2390 +/- ##
===========================================
- Coverage 98.08% 98.06% -0.03%
===========================================
Files 249 255 +6
Lines 7194 7169 -25
Branches 2066 2071 +5
===========================================
- Hits 7056 7030 -26
- Misses 128 129 +1
Partials 10 10 ☔ View full report in Codecov by Sentry. |
|
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
🧹 Outside diff range and nitpick comments (1)
.github/workflows/issue.yml (1)
29-55
: Add error handling to the script.Consider wrapping the script in a try-catch block to handle potential API errors gracefully.
Add error handling:
script: | + try { const { owner, repo } = context.repo; // ... rest of the script ... + } catch (error) { + core.setFailed(`Action failed with error: ${error}`); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/issue.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/issue.yml (1)
21-25
: LGTM! Efficient checkout configuration.
The sparse checkout is well configured to only fetch the required configuration file.
- uses: actions/github-script@v6 | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
with: | ||
script: | | ||
const { owner, repo } = context.repo; | ||
const issue_number = context.issue.number; | ||
const apiParams = { | ||
owner, | ||
repo, | ||
issue_number | ||
}; | ||
const labels = await github.rest.issues.listLabelsOnIssue(apiParams); | ||
if(labels.data.reduce((a, c)=>a||["dependencies"].includes(c.name), false)) | ||
await github.rest.issues.addLabels({ | ||
owner: context.repo.owner, | ||
repo: context.repo.repo, | ||
issue_number: context.issue.number, | ||
labels: ["good first issue", "security"] | ||
}); | ||
else if(labels.data.reduce((a, c)=>a||["security", "ui/ux"].includes(c.name), false)) | ||
await github.rest.issues.addLabels({ | ||
owner: context.repo.owner, | ||
repo: context.repo.repo, | ||
issue_number: context.issue.number, | ||
labels: ["good first issue"] | ||
}); |
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.
Implementation doesn't match requirements.
The current implementation has several issues:
- It checks existing labels instead of scanning the issue body for keywords
- The label combinations don't match the requirements
- It doesn't handle all required keyword cases
- There's potential for duplicate label additions
Here's the corrected implementation:
- uses: actions/github-script@v6
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
script: |
const { owner, repo } = context.repo;
const issue_number = context.issue.number;
const apiParams = {
owner,
repo,
issue_number
};
- const labels = await github.rest.issues.listLabelsOnIssue(apiParams);
- if(labels.data.reduce((a, c)=>a||["dependencies"].includes(c.name), false))
- await github.rest.issues.addLabels({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: context.issue.number,
- labels: ["good first issue", "security"]
- });
- else if(labels.data.reduce((a, c)=>a||["security", "ui/ux"].includes(c.name), false))
- await github.rest.issues.addLabels({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: context.issue.number,
- labels: ["good first issue"]
- });
+ // Get issue details
+ const issue = await github.rest.issues.get(apiParams);
+ const body = issue.data.body?.toLowerCase() || '';
+
+ // Initialize labels set with the default label
+ const labelsToAdd = new Set(['good first issue']);
+
+ // Check for dependencies keywords
+ if (body.match(/\b(dependabot|dependency|dependencies)\b/)) {
+ labelsToAdd.add('dependencies').add('security');
+ }
+
+ // Check for security keyword
+ if (body.match(/\bsecurity\b/)) {
+ labelsToAdd.add('security');
+ }
+
+ // Check for UI/UX keywords
+ if (body.match(/\b(layout|screen|design|figma)\b/)) {
+ labelsToAdd.add('ui/ux');
+ }
+
+ // Add labels if we have more than just the default label
+ if (labelsToAdd.size > 1) {
+ await github.rest.issues.addLabels({
+ ...apiParams,
+ labels: Array.from(labelsToAdd)
+ });
+ }
Key improvements:
- Scans issue body for keywords as per requirements
- Handles all required keyword-label mappings
- Uses Set to prevent duplicate labels
- Adds null check for issue body
- Reuses apiParams object
📝 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.
- uses: actions/github-script@v6 | |
env: | |
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
with: | |
script: | | |
const { owner, repo } = context.repo; | |
const issue_number = context.issue.number; | |
const apiParams = { | |
owner, | |
repo, | |
issue_number | |
}; | |
const labels = await github.rest.issues.listLabelsOnIssue(apiParams); | |
if(labels.data.reduce((a, c)=>a||["dependencies"].includes(c.name), false)) | |
await github.rest.issues.addLabels({ | |
owner: context.repo.owner, | |
repo: context.repo.repo, | |
issue_number: context.issue.number, | |
labels: ["good first issue", "security"] | |
}); | |
else if(labels.data.reduce((a, c)=>a||["security", "ui/ux"].includes(c.name), false)) | |
await github.rest.issues.addLabels({ | |
owner: context.repo.owner, | |
repo: context.repo.repo, | |
issue_number: context.issue.number, | |
labels: ["good first issue"] | |
}); | |
- uses: actions/github-script@v6 | |
env: | |
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
with: | |
script: | | |
const { owner, repo } = context.repo; | |
const issue_number = context.issue.number; | |
const apiParams = { | |
owner, | |
repo, | |
issue_number | |
}; | |
// Get issue details | |
const issue = await github.rest.issues.get(apiParams); | |
const body = issue.data.body?.toLowerCase() || ''; | |
// Initialize labels set with the default label | |
const labelsToAdd = new Set(['good first issue']); | |
// Check for dependencies keywords | |
if (body.match(/\b(dependabot|dependency|dependencies)\b/)) { | |
labelsToAdd.add('dependencies').add('security'); | |
} | |
// Check for security keyword | |
if (body.match(/\bsecurity\b/)) { | |
labelsToAdd.add('security'); | |
} | |
// Check for UI/UX keywords | |
if (body.match(/\b(layout|screen|design|figma)\b/)) { | |
labelsToAdd.add('ui/ux'); | |
} | |
// Add labels if we have more than just the default label | |
if (labelsToAdd.size > 1) { | |
await github.rest.issues.addLabels({ | |
...apiParams, | |
labels: Array.from(labelsToAdd) | |
}); | |
} |
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.
The changes suggested by the coderabbit will add 'good first issue' to every issue by default.
@palisadoes
and my approach is not to use script for that but to use for detecting labels as mentioned in PR Body.
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
317dd68
into
PalisadoesFoundation:develop
What kind of change does this PR introduce?
feature about issue workflows
**Issue Number: **
Fixes #2256
Did you add tests for your changes?
No
Snapshots/Videos:
Results:
If relevant, did you update the documentation?
Summary
As described in issue #2256 the workflows will be able to add automatic labels for following conditions
Dependencies: When the words dependabot, dependency or dependencies are in the body of the issue Add the labels dependencies, security and good first issue
Security: When the word security are in the body of the issue Add the label security and good first issue
Design: When the words layout, screen, design or figma are in the body of the issue Add the label ui/ux and good first issue
Does this PR introduce a breaking change?
Other information
Added a extra step in the given
issue.yml
file which uses actions/github-script@v6 for solving following problemProblem : We can not have same synonym for multiple labels if we add same synonym in multiple if we add same synonym in multiple lists of synonyms then it adds only single label in latest list refer auto-lable/issue-93
Soultution By me:
security
for same synonyms fromdependencies
and synonymsecurity
: we'll have only security as synonym in thelabelsSynonyms
and for same synonyms fromdependencies
we'll have github script to add the labels by detecting ifdependencies
is label to issuegood first issue
if we have anyone label fromdependencies
,security
,ui/ux
assigned to issueHave you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Bug Fixes
Chores