# Highfive rotation

**URL:** <https://internals.rust-lang.org/t/highfive-rotation/11865>\
**Category:** Uncategorized\
**Created:** [February 21, 2020, 3:12am UTC](https://internals.rust-lang.org/t/highfive-rotation/11865 "2020-02-21T03:12:30Z")\
**Posts on this page:** 15\
**Page:** 1

<div class="post-metadata">

**Author:** ![ehuss](https://avatars.discourse-cdn.com/v4/letter/e/9de0a6/32.png) [@ehuss](https://internals.rust-lang.org/u/ehuss)\
**Post date:** [February 21, 2020, 3:12am UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/1 "2020-02-21T03:12:30Z")

</div>

Niko is the only person on the highfive review list for a large percentage of the rust-lang/rust repository. I wanted to open up a conversation about possibly finding more people to cover the repository. I don't think there is a major problem right now, but I think it might make things run more smoothly if we were clearer about who is responsible for different parts of the repo.

AFAIK, currently Niko is the only (automatic) reviewer for:

- Everything in the root directory (Cargo.lock, Cargo.toml, etc.)
- src/bootstrap
- src/build\_helper
- src/etc
- src/libpanic\_abort
- src/libpanic\_unwind
- src/libproc\_macro
- src/libprofiler\_builtins
- src/libunwind
- src/llvm-project
- src/rtstartup
- src/rustc
- src/rustllvm
- src/stage0.txt
- src/stdarch
- src/tools/ — a bunch of things here, including:
- src/tools/cargo
- src/tools/clippy
- src/tools/compiletest
- src/tools/miri
- src/tools/rls
- src/tools/rust-installer
- src/tools/rustbook
- src/tools/rustfmt
- src/tools/tidy

I personally would like to know who should review things for `bootstrap`, `tidy`, lockfile updates, and license changes since I need to modify them occasionally. I sorta know who, but I don't want to bug the wrong people, or have people approve things that they probably shouldn't.

A problem I'd like to discuss is the feeling of being saddled with responsibility if one is added to the highfive list. I am already over-subscribed, so I'm reluctant to add myself to the rotation because I'm already ignoring too many things now.

One rough idea I had was to make an option in highfive to have a time-limited signup. Like I could sign up for a directory for anywhere from 1 month to a year, and then it could automatically remove me. That might help with the feeling of getting "stuck" with something. But I'm not sure if that would actually be beneficial.

Maybe others have ideas on how to make this process better? Are there people or groups that can be added to the directories listed above?

---

<div class="post-metadata">

**Author:** ![petrochenkov](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/petrochenkov/32/2784_2.png) [@petrochenkov](https://internals.rust-lang.org/u/petrochenkov)\
**Post date:** [February 21, 2020, 6:07am UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/2 "2020-02-21T06:07:43Z")

</div>

> [@ehuss](#):
>
> src/libproc\_macro

I can take this.

Many entries in the list are low-volume, if they don't have an owner it may make sense to assign them to people from triage team who will then ping people who they think are appropriate for reviewing and reassign to them.  
That should provide better reaction times, ~day instead of ~week.

---

<div class="post-metadata">

**Author:** ![kinggoesgaming](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kinggoesgaming/32/2208_2.png) [@kinggoesgaming](https://internals.rust-lang.org/u/kinggoesgaming)\
**Post date:** [February 21, 2020, 6:15am UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/3 "2020-02-21T06:15:23Z")

</div>

> [@ehuss](#):
>
> `src/tools/rust-installer`

Can I take this?

Fairly low volume and fits with me

---

<div class="post-metadata">

**Author:** ![josh](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/josh/32/5934_2.png) [@josh](https://internals.rust-lang.org/u/josh)\
**Post date:** [February 21, 2020, 7:27am UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/4 "2020-02-21T07:27:55Z")

</div>

I would be happy to review changes to root-directory files and license files. And since I'm on the cargo team, I'd also be happy to be on highfive rotation for src/tools/cargo.

---

<div class="post-metadata">

**Author:** ![cuviper](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cuviper/32/1897_2.png) [@cuviper](https://internals.rust-lang.org/u/cuviper)\
**Post date:** [February 21, 2020, 7:42am UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/5 "2020-02-21T07:42:21Z")

</div>

I can help with `src/llvm-project` and `src/rustllvm`, at least.

---

<div class="post-metadata">

**Author:** ![Mark\_Simulacrum](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mark_simulacrum/32/2853_2.png) [@Mark\_Simulacrum](https://internals.rust-lang.org/u/Mark_Simulacrum)\
**Post date:** [February 21, 2020, 12:21pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/6 "2020-02-21T12:21:57Z")

</div>

I can take:

- Everything in the root directory (Cargo.lock, Cargo.toml, etc.)
- src/bootstrap
- src/build\_helper
- src/etc
- src/stage0.txt
- src/tools/compiletest
- src/tools/rust-installer (though I think it's a submodule?)
- src/tools/tidy

These should be libs under current rules I believe:

- src/libpanic\_abort
- src/libpanic\_unwind
- src/stdarch (note, I believe this is a submodule)

---

<div class="post-metadata">

**Author:** ![petrochenkov](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/petrochenkov/32/2784_2.png) [@petrochenkov](https://internals.rust-lang.org/u/petrochenkov)\
**Post date:** [February 21, 2020, 12:49pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/7 "2020-02-21T12:49:54Z")

</div>

> [@Mark\_Simulacrum](#):
>
> though I think it's a submodule

As a result submodule update PRs are assigned to nikomatsakis, see e.g. [submodules: update clippy from b91ae16e to 2855b214 by matthiaskrgr · Pull Request #69278 · rust-lang/rust · GitHub](https://github.com/rust-lang/rust/pull/69278) 🙂

---

<div class="post-metadata">

**Author:** ![Mark\_Simulacrum](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mark_simulacrum/32/2853_2.png) [@Mark\_Simulacrum](https://internals.rust-lang.org/u/Mark_Simulacrum)\
**Post date:** [February 21, 2020, 1:02pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/8 "2020-02-21T13:02:57Z")

</div>

Yes, my point is that submodule updates _usually_ need less scrutiny (presuming they're not changing public details of our implementation in a way that needs e.g. lang signoff).

---

<div class="post-metadata">

**Author:** ![mark-i-m](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mark-i-m/32/3167_2.png) [@mark-i-m](https://internals.rust-lang.org/u/mark-i-m)\
**Post date:** [February 21, 2020, 4:40pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/9 "2020-02-21T16:40:27Z")

</div>

Perhaps something that could relieve the stress would be assigning more than one person to a PR, and they can agree between themselves who has the most bandwidth to review the PR... i.e. for each new PR, reviewers A and B would be assigned by highfive-bot and only one of the needs to review it.

A modest extension of this would be to assign the PR to a team with the expectation that somebody on the team reviews it.

Of course, I'm not on the highfive rotation at all, so maybe an actual reviewer might feel differently...

---

<div class="post-metadata">

**Author:** ![pietroalbini](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pietroalbini/32/10694_2.png) [@pietroalbini](https://internals.rust-lang.org/u/pietroalbini)\
**Post date:** [February 21, 2020, 4:59pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/10 "2020-02-21T16:59:33Z")

</div>

> [@mark-i-m](#):
>
> Perhaps something that could relieve the stress would be assigning more than one person to a PR, and they can agree between themselves who has the most bandwidth to review the PR... i.e. for each new PR, reviewers A and B would be assigned by highfive-bot and only one of the needs to review it.

My worry with this is that then both of them would assume the other will review the PR.

---

<div class="post-metadata">

**Author:** ![Centril](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/centril/32/3334_2.png) [@Centril](https://internals.rust-lang.org/u/Centril)\
**Post date:** [February 21, 2020, 6:11pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/11 "2020-02-21T18:11:04Z")

</div>

> [@Mark\_Simulacrum](#):
>
> These should be libs under current rules I believe:
> 
> - src/libpanic\_abort
> - src/libpanic\_unwind
> - src/stdarch (note, I believe this is a submodule)

(and lang, in particular for the last one)

> [@ehuss](#):
>
> src/tools/compiletest

I've done / reviewed some things here, so I could be added to this.

---

<div class="post-metadata">

**Author:** ![josh](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/josh/32/5934_2.png) [@josh](https://internals.rust-lang.org/u/josh)\
**Post date:** [February 21, 2020, 9:19pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/12 "2020-02-21T21:19:29Z")

</div>

> **[Diffusion of responsibility](https://en.wikipedia.org/wiki/Diffusion_of_responsibility)**
>
> Diffusion of responsibility is a sociopsychological phenomenon whereby a person is less likely to take responsibility for action or inaction when others are present. Considered a form of attribution, the individual assumes that others either are responsible for taking action or have already done so. 
> The diffusion of responsibility refers to the decreased responsibility of action each member of a group feels when she or he is part of a group. For example, in emergency situations, individuals fee...

highfive's assignment of _one_ person to a PR is excellent for avoiding this.

That said, it'd be nice if there were a way to re-invoke highfive, to pull in an additional reviewer if something feels complex.

---

<div class="post-metadata">

**Author:** ![mark-i-m](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mark-i-m/32/3167_2.png) [@mark-i-m](https://internals.rust-lang.org/u/mark-i-m)\
**Post date:** [February 21, 2020, 10:04pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/13 "2020-02-21T22:04:31Z")

</div>

Yeah, I thought that might be the case. An alternate might be to have two reviewers and make them both mandatory so that they "egg each other on" in some sense... but I'm not sure that would help with the original problem of stress...

---

<div class="post-metadata">

**Author:** ![cuviper](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cuviper/32/1897_2.png) [@cuviper](https://internals.rust-lang.org/u/cuviper)\
**Post date:** [February 21, 2020, 10:14pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/14 "2020-02-21T22:14:39Z")

</div>

> [@josh](#):
>
> That said, it'd be nice if there were a way to re-invoke highfive, to pull in an additional reviewer if something feels complex.

There's an issue:

> <https://github.com/rust-lang/highfive/issues/241>
>
> When a PR is first opened, highfive randomly assigns its review to someone. Base…d on https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust.json#L10 it looks like this is based on what files/directories the PR modifies. This is all good.
> 
> Sometimes, for a variety of possible reasons, the review an already-open PR needs to be reassigned to someone else. Typically what happens is that the person doing this navigates the appropriate team’s page under https://www.rust-lang.org/governance/ and picks someone at semi-random. It would be nice to have a way to ask highfive to auto-assign again, like it did when the PR was first opened:
> 
> \* This is fewer steps for the person who decided that reassigning was needed
> \* In some cases we have deliberate differences between team membership and auto-assigned reviewers
> 
> Ideally highfive would in that case pick someone \*different\* from the first time on that particular PR. However running the same algorithm again would be good already: there are usually enough reviewers to choose from that picking the same twice in a row is unlikely, and if it happens we can simply give the command again.
> 
> The exact command to trigger this is less important. I initially though of \`r? @rust-highfive\` (we "assign" to highfive the responsibility to reassign someone else), https://github.com/rust-lang/highfive/issues/189 proposed \`r? \_retry\` or \`r? \_\`

---

<div class="post-metadata">

**Author:** ![system](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/system/32/14092_2.png) [@system](https://internals.rust-lang.org/u/system)\
**Post date:** [May 21, 2020, 10:14pm UTC](https://internals.rust-lang.org/t/highfive-rotation/11865/15 "2020-05-21T22:14:39Z")

</div>

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.
