feat: Add reviewer on pull request based on commit history #2234
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @zakisk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances the automated pull request management system by introducing a new capability to intelligently suggest and assign reviewers. By analyzing the commit history of modified files, the system can now identify the most relevant contributor to review the changes, thereby streamlining the review process and ensuring appropriate expertise is involved.
Highlights
- Renaming of Automation Components: The
pr-labelerTekton PipelineRun and its associated Python scriptpr-labeler.pyhave been renamed topr-triagerandpr-triager.pyrespectively, reflecting a broader scope of automated PR management. - Automated Reviewer Assignment: A new feature has been introduced to automatically suggest and assign a reviewer to a pull request if no reviewers are already assigned. This is based on the commit history of the files changed in the PR.
- Reviewer Suggestion Logic: The reviewer suggestion process identifies the most frequent contributor (excluding the PR author) to the changed files. It prioritizes non-vendor files, processes up to the first 50 relevant files, and analyzes up to 20 commits per file to determine potential reviewers.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
added functionality in pr-triager.yaml PipelineRun to add reviewer in a pull request based on previous commit history Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
zakisk
force-pushed
the
feat-add-reviewer-on-pr
branch
from
0e0b795 to
b677b48
Compare
| // Patch the PipelineRun with the annotations and labels required by the watcher. | ||
| // The watcher reconciles only PipelineRuns that contain the state annotation, | ||
| // so we set it here to ensure reconciliation continues. |
There was a problem hiding this comment.
this is just for testing because we add reviewer based on commit history of the file but as pr-triager.yaml is appeared in this PR first time there is no history for the file and thus no reviewer getting assigned
There was a problem hiding this comment.
@chmouel you're the top most committer of the pipelineascode.go in last 20 commits 😄 that's why you're requested review by the pr-triager.yaml PipelineRun.
There was a problem hiding this comment.
yeah proibably to a lot of places in pac :)
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to automatically suggest and assign a reviewer to a pull request based on the commit history of the changed files. The implementation involves renaming pr-labeler to pr-triager to better reflect its new responsibilities, and adding new functions to handle the reviewer suggestion logic.
The logic for identifying a suitable reviewer by analyzing file contributors is sound. My main feedback is on improving code maintainability by replacing magic numbers with named constants and simplifying some of the logic for clarity. Overall, this is a valuable addition.
| print(f"Response: {e.response.text}") | ||
|
|
||
|
|
||
| def handle_reviewer_suggestions(pr_info, files_changed): |
There was a problem hiding this comment.
This function uses magic numbers 50 (line 267) and 20 (line 277). It's a good practice to define these as constants at the top of the file to improve readability and maintainability. For example:
MAX_FILES_TO_PROCESS = 50
MAX_COMMITS_PER_FILE = 20Then, you can use these constants within the function.
| files_to_process = non_vendor_files | ||
| if len(non_vendor_files) > 50: | ||
| print( | ||
| "PR has more than 50 non-vendor files, analyzing the first 50 for reviewer suggestion." | ||
| ) | ||
| files_to_process = non_vendor_files[:50] |
There was a problem hiding this comment.
The logic for limiting the number of files to process can be simplified to avoid reassigning files_to_process. The current implementation assigns to files_to_process and then potentially re-assigns it inside the if block. A clearer approach is to use an if/else block.
| files_to_process = non_vendor_files | |
| if len(non_vendor_files) > 50: | |
| print( | |
| "PR has more than 50 non-vendor files, analyzing the first 50 for reviewer suggestion." | |
| ) | |
| files_to_process = non_vendor_files[:50] | |
| if len(non_vendor_files) > 50: | |
| print( | |
| "PR has more than 50 non-vendor files, analyzing the first 50 for reviewer suggestion." | |
| ) | |
| files_to_process = non_vendor_files[:50] | |
| else: | |
| files_to_process = non_vendor_files |
|
more than i think about it maybe we should move it to another script instead, we can have all those tools in ./hack/ci or this would not scale if we start to add every tools that act on pr in a single script, we can reuse the duplicate functions via a tiny library.. |
📝 Description of the Change
added functionality in pr-triager.yaml PipelineRun to add reviewer in a pull request based on previous commit history
🔗 Linked GitHub Issue
Fixes #
👨🏻 Linked Jira
🚀 Type of Change
fix:)feat:)feat!:,fix!:)docs:)chore:)refactor:)enhance:)deps:)🧪 Testing Strategy
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.