Add automated PR reviewer rotation system #1491
Conversation
gursewak1997
commented
Aug 6, 2025
gursewak1997
commented
- Add GitHub Action workflow for automatic reviewer assignment
- Create Python script that reads reviewers from MAINTAINERS.md
- Implement 3-week sprint-based rotation cycle
- Preserve existing reviewer assignments (manual/GitHub auto)
- Add configuration for start date and rotation cycle
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an automated reviewer rotation system, which is a great addition. The implementation is mostly solid, but I've identified a critical bug in the reviewer selection logic that would prevent it from working as intended. There are also a few high-severity issues related to error handling that could cause the script to crash unexpectedly. My review provides specific suggestions to fix these problems and improve the script's robustness and maintainability.
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
2 times, most recently
from
d6b10e8 to
97110e7
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces an automated reviewer rotation system, which is a great addition. The implementation is solid, with a Python script handling the logic and a YAML file for configuration. My review focuses on improving the script's robustness, error handling, and code structure. I've suggested making the reviewer extraction from MAINTAINERS.md more resilient to format changes, using stderr for error messages, avoiding broad exception clauses, and refactoring to prevent redundant calculations. These changes will make the script more maintainable and reliable.
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
97110e7 to
f2ebaf6
Compare
There was a problem hiding this comment.
Code Review
The pull request introduces an automated PR reviewer rotation system using a Python script and a GitHub Action workflow. The script reads reviewers from MAINTAINERS.md, calculates the current reviewer based on a 3-week sprint-based rotation cycle, and assigns the reviewer to the PR. The configuration includes a start date and rotation cycle. I've identified some areas for improvement, including precision in date calculations and error handling in subprocess calls.
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
f2ebaf6 to
c90b7a8
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces an automated reviewer rotation system, which is a great initiative. The implementation is mostly solid, with a Python script to handle the logic and a YAML file for configuration. I've identified a couple of areas for improvement to make the script more robust and ensure correct error handling. Specifically, the method for parsing reviewers from MAINTAINERS.md could be made more resilient to file changes, and the error handling for when the gh CLI is not found should be made consistent to prevent silent failures in the CI/CD pipeline. My detailed comments and suggestions are below.
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
c90b7a8 to
c2cd2d0
Compare
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
c2cd2d0 to
4e031c0
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces an automated reviewer rotation system, which is a great initiative. The implementation includes a Python script to calculate the current reviewer based on a configurable rotation schedule and a MAINTAINERS.md file. The code is well-structured, but I have identified a few areas for improvement regarding robustness and correctness. My main concerns are the fragile parsing of the maintainers file and the use of timezone-naive datetimes, which could lead to incorrect behavior. Additionally, the error handling could be improved by raising exceptions from functions instead of calling sys.exit() directly. This would make the functions more reusable and testable, and centralize exit logic in the main function. I've provided specific suggestions to address these points.
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
4e031c0 to
0cfedc8
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces an automated reviewer rotation system using a Python script and a GitHub Action. The script reads reviewers from MAINTAINERS.md and assigns them based on a configurable weekly rotation. The implementation is well-structured, but I've identified a critical bug in the date handling that will cause the script to crash, and a medium-severity issue regarding the error handling flow that could cause confusion when debugging. My review includes suggestions to fix these issues.
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
0cfedc8 to
551a0cc
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces an automated reviewer rotation system, which is a great addition for managing PRs. The implementation consists of a Python script to handle the logic and a YAML file for configuration.
My review focuses on improving the maintainability and robustness of the Python script. I've suggested two main improvements:
- Refactoring the duplicated code for calling the
ghCLI tool into a shared helper function. - Making the regular expression for parsing usernames from
MAINTAINERS.mdmore specific to improve its reliability.
The overall approach is solid, and the script includes good error handling and clear logging. The changes are well-structured.
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
551a0cc to
d5145ec
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces an automated reviewer rotation system using a Python script and a GitHub Action. The implementation is solid, with good use of type hints and error handling. My feedback focuses on improving code clarity and maintainability by removing redundant code, clarifying a function's return type, and replacing a magic number with a more descriptive approach.
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
d5145ec to
3201ff0
Compare
|
/gemini review |
|
(Probably worth having a local AI agent do the review before putting up PRs, I only sometimes remember do that) |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
I do use the local agent, but it keeps coming up with suggestions even after initial feedback. I’m not sure it’s the right approach; it never seems fully convinced, and it’s a bit frustrating that it doesn’t provide all its suggestions in one go. |
|
The standard way I test Github Actions is to push to my fork's main branch; did you try that? |
Spent some time on this only to realize that GitHub doesn’t allow requesting reviewers on PRs within your own fork; not even yourself, which makes testing a bit tricky.
|
|
So recently I created https://github.com/bootc-dev/ci-sandbox/ which I was using to test some GHA stuff but it's intended for stuff like this, do you have write access there? |
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
3201ff0 to
2c43bf7
Compare
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
5 times, most recently
from
5405f25 to
13b1afe
Compare
- Auto-assign reviewers from MAINTAINERS.md on 3-week rotation - Exclude PR author from self-assignment - Trigger on PR open, ready-for-review, and pushes - Preserve existing manual reviewer assignments
gursewak1997
force-pushed
the
feature/auto-reviewer-rotation
branch
from
13b1afe to
efe6031
Compare
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Still just a superficial skim, I'm ok trying this out...
| types: [opened, ready_for_review] | ||
|
|
||
| permissions: | ||
| pull-requests: write |
There was a problem hiding this comment.
I think this is OK as is, though Chris added a Github app for the releases so that's another option for these kinds of workflows too.
| ['api', f'repos/{repo}/pulls/{pr_number}', '--jq', '.requested_reviewers[].login'], | ||
| f"Could not fetch existing reviewers for PR {pr_number}" | ||
| ) | ||
| return result.stdout.strip().split('\n') if result.stdout.strip() else [] |
There was a problem hiding this comment.
Not that it matters at all but just skimming this stood out to me, it's inefficient to allocate a string, trim it (allocating a whole new copy) and then re-trim again and allocate again but anyways, it doesn't matter
|
Let's try it out! Can always iterate from here |
|
This seems to be failing https://github.com/bootc-dev/bootc/actions/runs/16933716266/job/47985179886?pr=1503 |
2 participants
