Skip to content
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

fix: refactor notify triager workflow #3403

Closed
wants to merge 12 commits into from
Prev Previous commit
Next Next commit
testing
sambhavgupta0705 committed Dec 9, 2024
commit 097ad181fe2ef0cd5b835c0e9b25f7b4aa49f559
4 changes: 2 additions & 2 deletions .github/workflows/notify-triager.yml
Copy link
Member

@anshgoyalevil anshgoyalevil Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was because of MD file changes

Original file line number Diff line number Diff line change
@@ -71,7 +71,7 @@ jobs:
echo "codeTriagers=$codeTriagers" >> $GITHUB_ENV
- name: Add Reviewers for code files
if: steps.check-merge-branch.outputs.isMergeCommit == 'false' && steps.non-md-pr-changes.outputs.any_changed == 'true'
if: steps.non-md-pr-changes.outputs.any_changed == 'true'
run: |
IFS=' ' read -r -a codeTriagers <<< "${{ env.codeTriagers }}"
reviewers=$(printf ', "%s"' "${codeTriagers[@]}")
@@ -86,7 +86,7 @@ jobs:
}"
- name: Add Reviewers for doc files
if: steps.check-merge-branch.outputs.isMergeCommit == 'false' && steps.md-pr-changes.outputs.any_changed == 'true'
if: steps.md-pr-changes.outputs.any_changed == 'true'
run: |
IFS=' ' read -r -a docTriagers <<< "${{ env.docTriagers }}"
reviewers=$(printf ', "%s"' "${docTriagers[@]}")