github-automation.py: Fix mis-indented statement #149653
Conversation
Ensure all reviewers who approved the original PR are properly notified and added as reviewers on backport PRs. Resolves: llvm#109429.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
There was a problem hiding this comment.
Looks like this was last touched in f33e927 which introduced this code (without the indent) after the migration to phabricator. The original code had the behavior that this patch restores, so I think this makes sense.
Please make sure one of the release managers (Tobias/Tom) reviews this though given they are the ones this will actually be impacting.
|
Ping |
|
I am unsure when this code is called? I think it's best if @tstellar have a look |
| continue | ||
| for pull in main_commit.get_pulls(): | ||
| for review in pull.get_reviews(): | ||
| if review.state != "APPROVED": |
There was a problem hiding this comment.
FWIW, we might also want to check the author associations as a possible follow-up. For example:
| if review.state != "APPROVED": | |
| if review.state != "APPROVED" or review.author_association not in ("COLLABORATOR", "CONTRIBUTOR", "MANNEQUIN", "MEMBER", "OWNER"): |
(untested)
This would align with what the PR greeter script does:
llvm-project/.github/workflows/new-prs.yml
Lines 31 to 35 in ecadd90
That way, users who don't have sufficient permissions to land a PR (but have left a review on the original PR) wouldn't be notified and added as reviewers on backport PRs.
There was a problem hiding this comment.
Btw if the greeter code looks strange, it's because there's a bug in GitHub. There's a long comment above that snippet to justify that. We have to check for every status other than the one we care about because GitHub never actually sets it. Fun :)
4 participants
Ensure all reviewers who approved the original PR are properly notified and added as reviewers on backport PRs.
Resolves: #109429.