Skip to content

Take review preferences into account when determining reviewers #1947

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Apr 18, 2025

This PR implements step 6 of #1753. When [assign.review_prefs] is configured in triagebot.toml, triagebot will now take into account the number of PRs already assigned to a reviewer, and their reviewer preferences.

Users can set their review preferences by sending a DM to triagebot on Zulip, and sending work set-pr-limit <max-prs> (this was implemented in #1919). When a reviewer already has N PRs assigned where N >= max-prs, they will not be considered for assignment.

Note that the way this is currently implemented, the assigned PR tracking only works for rust-lang/rust, so it should not be enabled on any other repo.

There are a couple of questions in the design that we should consider (they do not necessarily block this PR though):

  • Should the assigned PR tracking be within a repository or across repositories? In other words, do reviewers want to configure a "max assigned PR limit" across the whole of rust-lang, or do they want to have a limit per repository?
  • Should we only consider open PRs or open + draft PRs to be assigned to a given reviewer?
  • Should we ignore this limit in certain situations, e.g. when you request a reviewer explicitly (r? reviewer) or when we use the adhoc group fallback assigment logic?

CC @apiraino

Requesting a review regarding the implementation details, but I still want to discuss the design on Zulip first and update Forge, so marking as a draft for now. Best reviewer commit-by-commit. I should probably add more logging before we merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant