fix: refactor notify triager workflow #3403
Conversation
|
Warning Rate limit exceeded@sambhavgupta0705 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 38 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 (1)
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
Documentation and Community
|
✅ 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% 70.18% +2.40%
==========================================
Files 21 21
Lines 664 664
==========================================
+ Hits 450 466 +16
+ Misses 214 198 -16 ☔ 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
.github/workflows/notify-triager.yml
Outdated
| 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
.github/workflows/notify-triager.yml
Outdated
| 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 |
There was a problem hiding this comment.
Hello there @sambhavgupta0705
I see you made some changes since the last changes, is this PR ready for review now?
There was a problem hiding this comment.
In the previous implementation of notify traiger workflow, we were getting doc triagers assigned to the PRs that didn't contain any doc changes. This was because of how the workflow was checking for file changes in the PR.
For example, in previous implementation:
- User A created a PR P1 with code changes. -> Only code triagers assigned to the PR
- User B created a PR P2 with doc changes. -> Only doc triagers assigned to the PR
- PR P2 gets merged first.
- PR P1 is updated to include a merge commit containing PR P2 changes -> Doc traigers also gets assigned to this PR P1. Now this is the part where we are going wrong. Also, this behavior is still ambigious
There was a problem hiding this comment.
I think it was because of MD file changes
|
Hey |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/notify-triager.yml (1)
Line range hint
1-15: Security: Review workflow permissions and checkout configurationThe workflow has potential security concerns:
- Using
pull_request_targetwith PR head checkout can expose repository secrets to malicious PRs- Downgrading
actions/checkoutfrom v4.1.1 to v4 might miss security fixesConsider these changes:
- Add explicit
permissionsblock to limit workflow scope- Use specific version v4.1.1 instead of v4
- Add input validation before using PR data
name: Notify Triagers on: pull_request_target: types: [opened, reopened, synchronize, edited, ready_for_review] +permissions: + pull-requests: write + contents: read jobs: Notify-triagers: runs-on: ubuntu-latest steps: - name: Checkout Repository - uses: actions/checkout@v4 + uses: actions/[email protected]🧰 Tools
🪛 actionlint (1.7.4)
19-19: shellcheck reported issue in this script: SC2086:info:2:44: Double quote to prevent globbing and word splitting
(shellcheck)
25-25: shellcheck reported issue in this script: SC2193:warning:1:35: The arguments to this comparison can never be equal. Make sure your syntax is correct
(shellcheck)
25-25: shellcheck reported issue in this script: SC2086:info:2:32: Double quote to prevent globbing and word splitting
(shellcheck)
25-25: shellcheck reported issue in this script: SC2086:info:4:33: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/notify-triager.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/notify-triager.yml
19-19: shellcheck reported issue in this script: SC2086:info:2:44: Double quote to prevent globbing and word splitting
(shellcheck)
25-25: shellcheck reported issue in this script: SC2193:warning:1:35: The arguments to this comparison can never be equal. Make sure your syntax is correct
(shellcheck)
25-25: shellcheck reported issue in this script: SC2086:info:2:32: Double quote to prevent globbing and word splitting
(shellcheck)
25-25: shellcheck reported issue in this script: SC2086:info:4:33: Double quote to prevent globbing and word splitting
(shellcheck)
54-54: shellcheck reported issue in this script: SC2086:info:2:38: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (2)
.github/workflows/notify-triager.yml (2)
48-57: Improve robustness of triager extraction
The previous review comment's solution for handling edge cases in triager extraction is still applicable here. The current awk implementation might miss edge cases with whitespace or line endings.
🧰 Tools
🪛 actionlint (1.7.4)
48-48: shellcheck reported issue in this script: SC2086:info:2:36: Double quote to prevent globbing and word splitting
(shellcheck)
54-54: shellcheck reported issue in this script: SC2086:info:2:38: Double quote to prevent globbing and word splitting
(shellcheck)
58-80:
Add error handling for GitHub API calls
The current implementation doesn't handle API failures, which could lead to silent failures.
Add error handling:
- name: Add Reviewers for code files
if: env.isMergeCommit == 'false' && steps.non-md-pr-changes.outputs.any_changed == 'true'
run: |
IFS=' ' read -r -a codeTriagers <<< "${{ env.codeTriagers }}"
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 code reviewers. Status code: $status_code"
+ exit 1
+ fiThe previous review comment's solution for reducing code duplication between doc and code reviewer assignment is still applicable here.
.github/workflows/notify-triager.yml
Outdated
| run: | | ||
| commit_message=$(git log --format=%B -n 1 ${{ github.event.pull_request.head.sha }}) | ||
| echo "commit_message=$commit_message" >> $GITHUB_OUTPUT | ||
| commit_message=$(git log --format=%B -n 1 "${{ github.event.pull_request.head.sha }}") | ||
| echo "commit_message=${commit_message}" >> $GITHUB_ENV | ||
|
|
||
| - name: Check if last commit is a merge commit | ||
| id: check-merge-branch | ||
| run: | | ||
| if [[ "${{ steps.commit-message.outputs.commit_message }}" == *"Merge branch"* ]]; then | ||
| echo "Last commit is a merge commit" | ||
| echo "isMergeCommit=true" >> $GITHUB_OUTPUT | ||
| if [[ "${{ env.commit_message }}" == *"Merge branch"* ]]; then | ||
| echo "isMergeCommit=true" >> $GITHUB_ENV | ||
| else | ||
| echo "Last commit message does not contain Merge branch" | ||
| echo "isMergeCommit=false" >> $GITHUB_OUTPUT | ||
| echo "isMergeCommit=false" >> $GITHUB_ENV | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve shell script robustness and merge detection
The shell script has several areas for improvement:
- Missing quotes around variables (flagged by shellcheck)
- Merge detection could miss other merge patterns
Apply these improvements:
- name: Get commit message
id: commit-message
run: |
- commit_message=$(git log --format=%B -n 1 "${{ github.event.pull_request.head.sha }}")
- echo "commit_message=${commit_message}" >> $GITHUB_ENV
+ commit_message="$(git log --format=%B -n 1 "${{ github.event.pull_request.head.sha }}")"
+ echo "commit_message=${commit_message}" >> "$GITHUB_ENV"
- name: Check if last commit is a merge commit
id: check-merge-branch
run: |
- if [[ "${{ env.commit_message }}" == *"Merge branch"* ]]; then
- echo "isMergeCommit=true" >> $GITHUB_ENV
+ if git rev-parse --verify "${{ github.event.pull_request.head.sha }}^2" >/dev/null 2>&1; then
+ echo "isMergeCommit=true" >> "$GITHUB_ENV"
else
- echo "isMergeCommit=false" >> $GITHUB_ENV
+ echo "isMergeCommit=false" >> "$GITHUB_ENV"
fi📝 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.
| run: | | |
| commit_message=$(git log --format=%B -n 1 ${{ github.event.pull_request.head.sha }}) | |
| echo "commit_message=$commit_message" >> $GITHUB_OUTPUT | |
| commit_message=$(git log --format=%B -n 1 "${{ github.event.pull_request.head.sha }}") | |
| echo "commit_message=${commit_message}" >> $GITHUB_ENV | |
| - name: Check if last commit is a merge commit | |
| id: check-merge-branch | |
| run: | | |
| if [[ "${{ steps.commit-message.outputs.commit_message }}" == *"Merge branch"* ]]; then | |
| echo "Last commit is a merge commit" | |
| echo "isMergeCommit=true" >> $GITHUB_OUTPUT | |
| if [[ "${{ env.commit_message }}" == *"Merge branch"* ]]; then | |
| echo "isMergeCommit=true" >> $GITHUB_ENV | |
| else | |
| echo "Last commit message does not contain Merge branch" | |
| echo "isMergeCommit=false" >> $GITHUB_OUTPUT | |
| echo "isMergeCommit=false" >> $GITHUB_ENV | |
| fi | |
| run: | | |
| commit_message="$(git log --format=%B -n 1 "${{ github.event.pull_request.head.sha }}")" | |
| echo "commit_message=${commit_message}" >> "$GITHUB_ENV" | |
| - name: Check if last commit is a merge commit | |
| id: check-merge-branch | |
| run: | | |
| if git rev-parse --verify "${{ github.event.pull_request.head.sha }}^2" >/dev/null 2>&1; then | |
| echo "isMergeCommit=true" >> "$GITHUB_ENV" | |
| else | |
| echo "isMergeCommit=false" >> "$GITHUB_ENV" | |
| fi |
🧰 Tools
🪛 actionlint (1.7.4)
19-19: shellcheck reported issue in this script: SC2086:info:2:44: Double quote to prevent globbing and word splitting
(shellcheck)
25-25: shellcheck reported issue in this script: SC2193:warning:1:35: The arguments to this comparison can never be equal. Make sure your syntax is correct
(shellcheck)
25-25: shellcheck reported issue in this script: SC2086:info:2:32: Double quote to prevent globbing and word splitting
(shellcheck)
25-25: shellcheck reported issue in this script: SC2086:info:4:33: Double quote to prevent globbing and word splitting
(shellcheck)
asyncapi-bot
requested review from
BhaswatiRoy,
J0SAL,
TRohit20 and
VaishnaviNandakumar
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/notify-triager.yml (1)
102-102: Fix file format issuesThe file has formatting issues:
- Missing newline at end of file
- Trailing spaces on line 102
- +🧰 Tools
🪛 yamllint (1.35.1)
[error] 102-102: no new line character at the end of file
(new-line-at-end-of-file)
[error] 102-102: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/notify-triager.yml(4 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/notify-triager.yml
[error] 102-102: no new line character at the end of file
(new-line-at-end-of-file)
[error] 102-102: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
.github/workflows/notify-triager.yml (3)
Line range hint 28-34: Use git rev-parse for reliable merge commit detection
The current string matching approach for detecting merge commits is unreliable. Using git rev-parse would be more accurate as it checks the actual commit structure.
- if [[ "${{ steps.commit-message.outputs.commit_message }}" == *"Merge branch"* ]]; then
- echo "Last commit is a merge commit"
- echo "isMergeCommit=true" >> $GITHUB_OUTPUT
- else
- echo "Last commit message does not contain Merge branch"
- echo "isMergeCommit=false" >> $GITHUB_OUTPUT
- fi
+ if git rev-parse --verify "${{ github.event.pull_request.head.sha }}^2" >/dev/null 2>&1; then
+ echo "Last commit is a merge commit"
+ echo "isMergeCommit=true" >> "$GITHUB_OUTPUT"
+ else
+ echo "Last commit is not a merge commit"
+ echo "isMergeCommit=false" >> "$GITHUB_OUTPUT"
+ fi🧰 Tools
🪛 actionlint (1.7.4)
19-19: shellcheck reported issue in this script: SC2086:info:5:42: Double quote to prevent globbing and word splitting
(shellcheck)
Line range hint 55-71: Improve triager extraction robustness
The current approach using tail and head is fragile as it depends on exact line positions. Let's make it more robust:
- name: Extract Doc Triage Maintainers
id: doc-triager
run: |
- docTriagers=$(grep '^#' CODEOWNERS | tail -n 2 | head -n 1)
- echo "docTriagers: $docTriagers"
- prefix="#docTriagers: "
- docTriagers=${docTriagers#$prefix}
- echo "docTriagers=$docTriagers" >> $GITHUB_ENV
+ if [ ! -f CODEOWNERS ]; then
+ echo "::error::CODEOWNERS file not found"
+ exit 1
+ fi
+ if ! docTriagers=$(grep -E '^\s*#docTriagers:' CODEOWNERS | sed 's/^\s*#docTriagers:\s*//' | tr -d '\r'); then
+ echo "::error::Failed to extract doc triagers"
+ exit 1
+ fi
+ echo "docTriagers=$docTriagers" >> "$GITHUB_ENV"
- name: Extract Code Triage Maintainers
id: code-triager
run: |
- codeTriagers=$(grep '^#' CODEOWNERS | tail -n 1)
- echo "codeTriagers: $codeTriagers"
- prefix="#codeTriagers: "
- codeTriagers=${codeTriagers#$prefix}
- echo "codeTriagers=$codeTriagers" >> $GITHUB_ENV
+ if ! codeTriagers=$(grep -E '^\s*#codeTriagers:' CODEOWNERS | sed 's/^\s*#codeTriagers:\s*//' | tr -d '\r'); then
+ echo "::error::Failed to extract code triagers"
+ exit 1
+ fi
+ echo "codeTriagers=$codeTriagers" >> "$GITHUB_ENV"Line range hint 74-102: Reduce duplication and add error handling in reviewer assignment
The reviewer assignment logic is duplicated and lacks error handling for the GitHub API calls.
+ - name: Setup reviewer assignment function
+ run: |
+ 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
+ }
- name: Add Reviewers for code files
if: steps.non-md-pr-changes.outputs.any_changed == 'true'
run: |
IFS=' ' read -r -a codeTriagers <<< "${{ env.codeTriagers }}"
- 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
- }"
+ add_reviewers "code" "${codeTriagers[@]}"
- name: Add Reviewers for doc files
if: steps.md-pr-changes.outputs.any_changed == 'true'
run: |
IFS=' ' read -r -a docTriagers <<< "${{ env.docTriagers }}"
- 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
- }"
+ add_reviewers "doc" "${docTriagers[@]}"| # Extract the commit message | ||
| commit_message=$(git log --format=%B -n 1 ${{ github.event.pull_request.head.sha }}) | ||
| commit_message=$(echo "$commit_message" | tr '\n' ' ') | ||
| commit_message=$(echo "$commit_message" | sed 's/[<>|]//g' | sed 's/[][]//g' | sed 's/(//g' | sed 's/)//g' | xargs) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve commit message extraction robustness and efficiency
The commit message extraction and cleaning can be improved for better robustness and efficiency.
- commit_message=$(git log --format=%B -n 1 ${{ github.event.pull_request.head.sha }})
- commit_message=$(echo "$commit_message" | tr '\n' ' ')
- commit_message=$(echo "$commit_message" | sed 's/[<>|]//g' | sed 's/[][]//g' | sed 's/(//g' | sed 's/)//g' | xargs)
+ if ! commit_message=$(git log --format=%B -n 1 "${{ github.event.pull_request.head.sha }}"); then
+ echo "::error::Failed to extract commit message"
+ exit 1
+ fi
+ commit_message=$(echo "$commit_message" | tr '\n' ' ' | sed 's/[<>|()[\]]//g' | xargs)
echo "commit_message=$commit_message" >> $GITHUB_OUTPUTChanges:
- Added quotes around GitHub SHA to prevent word splitting
- Added error handling for git log command
- Combined multiple sed commands into one for better efficiency
📝 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.
| # Extract the commit message | |
| commit_message=$(git log --format=%B -n 1 ${{ github.event.pull_request.head.sha }}) | |
| commit_message=$(echo "$commit_message" | tr '\n' ' ') | |
| commit_message=$(echo "$commit_message" | sed 's/[<>|]//g' | sed 's/[][]//g' | sed 's/(//g' | sed 's/)//g' | xargs) | |
| # Extract the commit message | |
| if ! commit_message=$(git log --format=%B -n 1 "${{ github.event.pull_request.head.sha }}"); then | |
| echo "::error::Failed to extract commit message" | |
| exit 1 | |
| fi | |
| commit_message=$(echo "$commit_message" | tr '\n' ' ' | sed 's/[<>|()[\]]//g' | xargs) | |
| echo "commit_message=$commit_message" >> $GITHUB_OUTPUT |
|
closing it for sometime as I am doing testing on this |
ref : #3214
Summary by CodeRabbit
Bug Fixes
Improvements
CODEOWNERSfile to modify owner listings and comment formatting.