Skip to content

Review rotation policy: remove reviewer from active review rotation if there's prolonged lack of feedback on randomly-rolled PRs / indication that unavailability is temporary #856

Closed
@jieyouxu

Description

@jieyouxu

Proposal

Motivation

Life happens. Sometimes compiler reviewers are just busy. Sometimes compiler reviewers on active compiler review rotation simply don't have enough time / don't feel like reviewing randomly rolled PRs, during extremely hectic / busy periods of time. And that's completely understandable.

At the same time, contributors submitting PRs for compiler changes can also reasonably expect that they will receive feedback within a reasonable timeframe, or at least indication that the randomly rolled compiler reviewer (on active review rotation) acknowledges the PR but may not get to it soon. It can be frustrating if the PR stays open for prolonged periods of time, yet receives no feedback from the randomly rolled compiler reviewer, accumulating merge conflicts.

See prior discussions in:

Proposed course of action

If a compiler reviewer on active compiler review rotation has not provided feedback on most randomly-rolled compiler PRs for more than 4 weeks, remove the busy/inactive compiler reviewer from active compiler review rotation. Note that this prevents randomly assigned compiler PRs but intentionally does not prevent explicit/manual r? assignment.

The busy/inactive compiler reviewer is of course more-than-welcomed to re-add themselves to the active compiler review rotation, when they have more availability.

This is a two-way door, because:

  1. It's also very easy for a compiler reviewer to re-add themselves back to the active compiler review rotation.
  2. We can also adjust the minimum threshold if in practice we find that 4 weeks is too short.

Removal protocol

  • Submit a PR to rust-lang/rust to remove the busy/inactive compiler reviewer from the active compiler review rotation in assign.adhoc_groups.compiler triagebot.toml config.
    • cc the busy/inactive compiler reviewer
    • Explicitly state that

      Please feel free to re-add yourself back to the active review rotation once you have more availability (if you feel like it).

Mentors or Reviewers

@Noratrieb who proposed this, but needs to have no blocking concerns from compiler team in general.

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Metadata

Metadata

Assignees

No one assigned

    Labels

    T-compilerAdd this label so rfcbot knows to poll the compiler teammajor-changeA proposal to make a major change to rustcmajor-change-acceptedA major change proposal that was accepted

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions