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

[Tracking] New pull request assignment proposal #1753

Open
4 of 8 tasks
apiraino opened this issue Dec 12, 2023 · 1 comment
Open
4 of 8 tasks

[Tracking] New pull request assignment proposal #1753

apiraino opened this issue Dec 12, 2023 · 1 comment
Assignees
Labels
A-assign-PR Area: PR auto assignment and welcome messages

Comments

@apiraino
Copy link
Contributor

apiraino commented Dec 12, 2023

This issue tracks the progress of the implementation of a new workflow for assigning pull requests to the Rust project contributors.

2024-02-20 UPDATE: After other opinions came in, a new plan was discussed. Updates below.

2023-12-12 UPDATE: After another meeting with T-infra, we agreed a second plan detailing how to split the work further. The new plan is detailed below.

2023-11-08 UPDATE: After a meeting with T-infra, it was remarked that the first implementation (#1719) was hard to review so we agreed a plan to split into smaller bites (#1745)

  • Step 1 (Workflow for tracking PRs assignment #1773 )
    • add the DB table, strip down DB fields to user_id,assigned_prs
    • Initial DB table population with a one-off job
    • no migration/import/sync tooling
  • Step 2 (Add Zulip integration to query PRs assignment #1778)
    • Implement a command to query from Zulip the number of assigned PRs.
  • Step 3 (Add webhook handler to update PR workload queues #1781)
  • Step 4 (Assign PR assignment based on work queue #1786 )
    • 2024-04-16: reverted in Revert "Assign PR assignment based on work queue" #1792 , parts of it will be submitted later
    • Add a new nullable field MAX_PR in the DB table
    • The starting value for MAX_PR is "null" i.e. no enforcement on PRs assignment
    • Update documentation on forge.r-l.o
  • Step 5 (Allow setting review assignment limit #1854 )
    • Modify the zulip triagebot command work show to also return MAX_PR
    • Add a Zulip command work set <num> to allow people updating their MAX_PR
    • Update documentation on forge.r-l.o
  • Step 6
    • Change the PRs assignment heuristic: filter out team members that have num_assigned_prs >= MAX_PR (with MAX_PR null will ignore this limit)
    • (?) Enable the new assignment if the a new key [review_prefs] in the triagebot.toml
  • Step X
    • MAX_PR will start to be enforced (add the setting it in the rust-lang/rust triagebot.toml)
    • People are then asked to set this value
    • if a PR is not assigned because everyone is at num_assigned_prs == MAX_PR, people can grab that PR manually
    • Self-assign respects MAX_PR either. Post a comment on the PR suggesting the user to increase their MAX_PR and self-assign again
  • Further work (some points to be clarified)
    • When an PR is closed/merged it stays assigned to a reviewer. Should be removed from the work queue returned by the Zulip triagebot
    • [Question] Add a filter to enable this feature only for members of a number of teams?
    • HTTP endpoint to sync assignment counts with GitHub
    • Tackle the preferences: HTML backoffice or chat-based? A web backoffice would be (imo) nice but the auth part is tricky to get approved (see comment).
    • how to sync PRs when/if bors webhook fail?
    • backoffice: add a button to the contributor to resync their own situation

Summary

The current pull request assignment is basically randomly assigned among team members. The new proposed workflow has the following features:

  • each Rust project contributor belonging to a team (I'd start will a small cohort of testers) can set their own review capacity and time off using a web backoffice
  • the web backoffice will persiste these data in a DB
  • everytime a pull request assignment is invoked with r? <team> or <team_member> the triagebot will check the current availability and workload of the candidates and assign the pull request to the team member less busy, excluding automatically those that are on time off in that moment.

A slightly old design document is at this HackMD link.

Permissions and Authorization

This backoffice could be thought as a simple form listing all team members. It is served by the triagebot (from triagebot.infra.rust-lang.org). Access to this backoffice is gated by a GitHub App. Users allowed to this backoffice:

  1. must be logged into GitHub
  2. must agree to the share some details with a GitHub App (only to read their profile)
  3. must be a Rust project team member
  4. the team they belong must be whitelisted in the env var NEW_PR_ASSIGNMENT_TEAMS

Team leaders are considered administrators and can see the preferences of every team member. Normal team members can only see their own preferences and those of team members that agree to share theirs within the team.

DEPLOYMENT

There will a few env vars to enable the new PR assignment and retrict access to this backoffice.

The idea would be to iterate on this pull request and iron out all the wrinkles visible without deployment:

  • Deploy this changes but keep the new PR assignment DISABLED. The web backoffice will still be accessible.
  • Team members can access that, start playing with with it fill their preferences
  • When we are all confident about these changes, we can flip the env variable USE_NEW_PR_ASSIGNMENT and have the team listed in NEW_PR_ASSIGNMENT_TEAMS use it for real.
  • After a first period of tests, add more teams
apiraino added a commit to apiraino/triagebot that referenced this issue Dec 13, 2023
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Dec 13, 2023
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Dec 13, 2023
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Dec 28, 2023
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Dec 28, 2023
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Dec 28, 2023
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Dec 28, 2023
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Dec 29, 2023
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Dec 29, 2023
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Jan 5, 2024
General overview at: rust-lang#1753

This patch implements the first part:
- a new DB table with just the fields to track how many PRs are
assigned to a contributor at any time
- Update this table everytime a PR is assigned or unassigned

No initial sync is planned at this time. Populating the table will happen over
time.
apiraino added a commit to apiraino/triagebot that referenced this issue Jan 5, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor at any time
- This tDB table will be updated everytime a PR is assigned or
  unassigned

No initial sync is planned at this time. DB table population will happen
over time automatically.
@ehuss ehuss added the A-assign-PR Area: PR auto assignment and welcome messages label Jan 21, 2024
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 20, 2024
- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 20, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 20, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 20, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 22, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 22, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 22, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 23, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 23, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 23, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
apiraino added a commit to apiraino/triagebot that referenced this issue Feb 23, 2024
General overview at: rust-lang#1753

- Added a new DB table with the fields to track how many PRs are
  assigned to a contributor
- Initial DB table population with a one-off job, manually run.
@apiraino
Copy link
Contributor Author

apiraino commented Apr 3, 2024

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assign-PR Area: PR auto assignment and welcome messages
Projects
None yet
Development

No branches or pull requests

2 participants