Rust: Bors policy question: Auto-reassignment on r+

[Centril]

Currently, the behavior of @bors is that whenever you @bors r+ or @bors r=UserFooBar, the reviewer doesn't change. However, this makes it less clear who the current reviewer is. For example, the assignee might not have even looked at the PR before someone else has r+ed it. I believe having an up to date assignee more frequently would simplify triage. Also, as @pnkfelix noted:

if one steals review action, it certainly seems like one should take on the burden of assignment as well.

While one could r? @Centril @bors r+ and such all the time, people often forget this and it instead causes churn to do so manually. As an example, there's https://github.com/rust-lang/rust/pull/59468.

Instead, I propose that:

• Whenever you r+ a PR, you get r? as well.
• If you r=UserFooBar, and @UserFooBar is not assigned, then they will be assigned.

The tagged teams are those that typically use @bors in this repo or perform triage and who will therefore be affected by this policy.

[Centril]

@rfcbot merge

[rfcbot]

Team member @Centril has proposed to merge this. The next step is review by the rest of the tagged team members:

• [x] @Amanieu
• [x] @ArazAbishov
• [x] @BatmanAoD
• [x] @BurntSushi
• [x] @Centril
• [x] @Dylan-DPC
• [x] @Kimundi
• [x] @KodrAus
• [x] @Mark-Simulacrum
• [x] @PramodBisht
• [x] @SimonSapin
• [x] @TimNN
• [x] @XAMPPRocky
• [x] @Zoxc
• [x] @aidanhs
• [x] @alexcrichton
• [ ] @ashleygwilliams
• [x] @aturon
• [x] @cramertj
• [x] @cuviper
• [x] @dtolnay
• [x] @eddyb
• [x] @erickt
• [x] @estebank
• [x] @jkordish
• [x] @jonas-schievink
• [x] @joshtriplett
• [x] @kennytm
• [x] @michaelwoerister
• [x] @nagisa
• [x] @nikomatsakis
• [x] @oli-obk
• [x] @petrochenkov
• [x] @pietroalbini
• [x] @pnkfelix
• [x] @scottmcm
• [x] @sfackler
• [x] @sgrif
• [x] @shepmaster
• [ ] @stokhos
• [x] @varkor
• [x] @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[estebank]

@rfcbot reviewed

[pietroalbini]

What should happen when the PR author approves the PR themselves (like when users are delegated and forget to r=reviewer, doing r+ instead)? I think we should skip reassigning if the person that'd be reassigned is the PR author.

Also, sometimes we do r=reviewer1,reviewer2, do we want to assign both in that case?

[kennytm]

Would the original reviewer be unassigned?

If we do @bors delegate=«user» would it assign the PR to «user» too?

[Centril]

I think we should skip reassigning if the person that'd be reassigned is the PR author.

Sure; seems fine... that's an implementation detail imo ;)

Also, sometimes we do r=reviewer1,reviewer2, do we want to assign both in that case?

Seems reasonable -- otherwise pick the first?

If we do @bors delegate=«user» would it assign the PR to «user» too?

Seems like @pietroalbini's note... so we'd skip in that case?

[shepmaster]

We cannot assign to users that are not members of the repo, correct?

[pietroalbini]

Nope, to be able to be assigned an user needs at least to have explicit read access to the repo.

[petrochenkov]

We should probably conservatively do nothing on @bors r=anything, anything not being already assigned is not a common situation.

[petrochenkov]

@Centril
Meta: I don't think you'll ever get a complete FCP from this many people.
(2/3 or 60% are used as a consensus sometimes.)

[Centril]

@petrochenkov hehe; maybe not... let's see ;) there have been similar situations that have worked.

As for r=foo, I think it would be fine to do nothing; I mostly care about r+.

[petrochenkov]

@Centril
There were also situations when similar changes were made with much less process overhead.

[kennytm]

Last time in #56951 the list has 26 people. For this there are 42 people. Let's see :)

---

IIRC r+ and r=foo share the same code path. Additionally, due to delegate=«user», the «user» sending r+ will have the same issue anyway. But assigning to a user outside the org is no-op anyway, I don't see a big problem as long as the original assignee is kept intact.

[Centril]

Ping, @ArazAbishov, @BatmanAoD, @BurntSushi, @PramodBisht, @TimNN, @aidanhs, @ashleygwilliams, @erickt, @stokhos, @withoutboats

[BatmanAoD]

@Centril Sorry, for some reason I mixed this up with your "test, please ignore" issue and thought review hadn't actually been requested from me. My bad!

[Centril]

Ping @ArazAbishov @stokhos @ashleygwilliams

[Centril]

Since there are only 3 checkboxes remaining and I've tried to reach those involved on many occasions I'm going to take the liberty to tick one of them (under the same "on leave" rationale) so that we can move on...

[Centril]

@rfcbot reviewed

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.