fix: refactor notify triager workflow #3403
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
CodeRabbit Configuration File (
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
sambhavgupta0705
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
|
This is currently in draft state |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3403 +/- ##
=======================================
Coverage 67.77% 67.77%
=======================================
Files 21 21
Lines 664 664
=======================================
Hits 450 450
Misses 214 214 ☔ View full report in Codecov by Sentry. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3403--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
.github/workflows/notify-triager.yml (1)
73-86: Add error handling for GitHub API callsThe current implementation doesn't handle API failures or rate limits.
if [ ${#codeTriagers[@]} -gt 0 ]; then reviewers=$(printf ', "%s"' "${codeTriagers[@]}") reviewers="[${reviewers:2}]" - curl \ - -X POST \ - -H "Authorization: token ${{ secrets.GH_TOKEN }}" \ - -H "Accept: application/vnd.github.v3+json" \ - https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers \ - -d "{ - \"reviewers\": $reviewers - }" + response=$(curl -s -w "%{http_code}" \ + -X POST \ + -H "Authorization: token ${{ secrets.GH_TOKEN }}" \ + -H "Accept: application/vnd.github.v3+json" \ + https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers \ + -d "{\"reviewers\": $reviewers}") + status_code=${response: -3} + if [ $status_code -ge 400 ]; then + echo "::error::Failed to add reviewers. Status code: $status_code" + exit 1 + fi fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/notify-triager.yml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/notify-triager.yml
62-62: shellcheck reported issue in this script: SC2086:info:5:38: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint
.github/workflows/notify-triager.yml
[error] 105-105: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/notify-triager.yml (1)
54-57: Verify CODEOWNERS file format and current triager assignments
Let's ensure the changes align with the current CODEOWNERS file structure.
Also applies to: 63-66
✅ Verification successful
The CODEOWNERS file format and triager assignments are correctly handled
The verification confirms that:
- The CODEOWNERS file exists and contains the expected triager definitions
- The
#docTriagersline is present with the correct format:#docTriagers: TRohit20 BhaswatiRoy VaishnaviNandakumar J0SAL - The code in the workflow correctly extracts triagers using
grep '^#docTriagers'andsed 's/#docTriagers: //'
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify CODEOWNERS file format and current triager assignments
# Check if CODEOWNERS file exists and show its content
if [ -f "CODEOWNERS" ]; then
echo "=== Current CODEOWNERS content ==="
cat CODEOWNERS
else
echo "CODEOWNERS file not found"
fi
# Check current triager line format
echo -e "\n=== Current triager definitions ==="
grep -E '^\s*#(doc|code)Triagers:' CODEOWNERS || echo "No triager definitions found"
Length of output: 1243
| docTriagers=$(grep '^#docTriagers' CODEOWNERS | sed 's/#docTriagers: //') | ||
| if [ -z "$docTriagers" ]; then | ||
| echo "No doc triagers found" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve robustness of triager extraction
The current implementation could be made more robust to handle edge cases:
- docTriagers=$(grep '^#docTriagers' CODEOWNERS | sed 's/#docTriagers: //')
+ docTriagers=$(grep -E '^\s*#docTriagers:' CODEOWNERS | sed 's/^\s*#docTriagers:\s*//' | tr -d '\r')- codeTriagers=$(grep '^#codeTriagers' CODEOWNERS | sed 's/#codeTriagers: //')
+ codeTriagers=$(grep -E '^\s*#codeTriagers:' CODEOWNERS | sed 's/^\s*#codeTriagers:\s*//' | tr -d '\r')Changes:
- Added
\s*to handle potential whitespace before/after the prefix - Added
tr -d '\r'to handle CRLF line endings - Used
-Efor extended regex support
Also applies to: 63-66
| if [ ${#docTriagers[@]} -gt 0 ]; then | ||
| reviewers=$(printf ', "%s"' "${docTriagers[@]}") | ||
| reviewers="[${reviewers:2}]" | ||
| curl \ | ||
| -X POST \ | ||
| -H "Authorization: token ${{ secrets.GH_TOKEN }}" \ | ||
| -H "Accept: application/vnd.github.v3+json" \ | ||
| https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers \ | ||
| -d "{ | ||
| \"reviewers\": $reviewers | ||
| }" | ||
| else | ||
| echo "No doc triagers found, skipping review assignment" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication and fix file format
- The reviewer assignment logic is duplicated between code and doc segments.
- File is missing a newline at the end.
Consider extracting the reviewer assignment logic into a reusable function:
+ add_reviewers() {
+ local reviewer_type=$1
+ local reviewers_array=("${@:2}")
+
+ if [ ${#reviewers_array[@]} -gt 0 ]; then
+ reviewers=$(printf ', "%s"' "${reviewers_array[@]}")
+ reviewers="[${reviewers:2}]"
+ response=$(curl -s -w "%{http_code}" \
+ -X POST \
+ -H "Authorization: token ${{ secrets.GH_TOKEN }}" \
+ -H "Accept: application/vnd.github.v3+json" \
+ https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers \
+ -d "{\"reviewers\": $reviewers}")
+ status_code=${response: -3}
+ if [ $status_code -ge 400 ]; then
+ echo "::error::Failed to add $reviewer_type reviewers. Status code: $status_code"
+ exit 1
+ fi
+ else
+ echo "No $reviewer_type triagers found, skipping review assignment"
+ fi
+ }
+
# Add code reviewers
IFS=' ' read -r -a codeTriagers <<< "${{ env.codeTriagers }}"
- if [ ${#codeTriagers[@]} -gt 0 ]; then
- # ... existing code ...
- fi
+ add_reviewers "code" "${codeTriagers[@]}"
# Add doc reviewers
IFS=' ' read -r -a docTriagers <<< "${{ env.docTriagers }}"
- if [ ${#docTriagers[@]} -gt 0 ]; then
- # ... existing code ...
- fi
+ add_reviewers "doc" "${docTriagers[@]}"
+📝 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 [ ${#docTriagers[@]} -gt 0 ]; then | |
| reviewers=$(printf ', "%s"' "${docTriagers[@]}") | |
| reviewers="[${reviewers:2}]" | |
| curl \ | |
| -X POST \ | |
| -H "Authorization: token ${{ secrets.GH_TOKEN }}" \ | |
| -H "Accept: application/vnd.github.v3+json" \ | |
| https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers \ | |
| -d "{ | |
| \"reviewers\": $reviewers | |
| }" | |
| else | |
| echo "No doc triagers found, skipping review assignment" | |
| fi | |
| add_reviewers() { | |
| local reviewer_type=$1 | |
| local reviewers_array=("${@:2}") | |
| if [ ${#reviewers_array[@]} -gt 0 ]; then | |
| reviewers=$(printf ', "%s"' "${reviewers_array[@]}") | |
| reviewers="[${reviewers:2}]" | |
| response=$(curl -s -w "%{http_code}" \ | |
| -X POST \ | |
| -H "Authorization: token ${{ secrets.GH_TOKEN }}" \ | |
| -H "Accept: application/vnd.github.v3+json" \ | |
| https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers \ | |
| -d "{\"reviewers\": $reviewers}") | |
| status_code=${response: -3} | |
| if [ $status_code -ge 400 ]; then | |
| echo "::error::Failed to add $reviewer_type reviewers. Status code: $status_code" | |
| exit 1 | |
| fi | |
| else | |
| echo "No $reviewer_type triagers found, skipping review assignment" | |
| fi | |
| } | |
| # Add code reviewers | |
| IFS=' ' read -r -a codeTriagers <<< "${{ env.codeTriagers }}" | |
| add_reviewers "code" "${codeTriagers[@]}" | |
| # Add doc reviewers | |
| IFS=' ' read -r -a docTriagers <<< "${{ env.docTriagers }}" | |
| add_reviewers "doc" "${docTriagers[@]}" | |
🧰 Tools
🪛 yamllint
[error] 105-105: no new line character at the end of file
(new-line-at-end-of-file)
|
@asyncapi/bounty_team |
|
@sambhavgupta0705 Is it ready for review? |
|
@anshgoyalevil nope |
ref : #3214
Summary by CodeRabbit
Bug Fixes
Improvements