Planned changes on the status graph

[timokau]

Trying to implement #23 has caused me to re-think the status labels. The intention from the beginning was to make PRs that need somebody (either any reviewer or a reviewer with commit bit) easy to find for the relevant people.

The needs_review and needs_work labels kind of do that, but they are not specific enough. We don't really care that much about the status of a PR which is currently in review. What do care about is which PRs are currently in need of a new reviewer.

Supporting this needs a bit of a more complicated state graph. Here's what I have come up with:

The two interesting states are (2) and (5) (highlighted in blue). They entirely unambiguously mark PRs that need somebody new to look at them. This makes them easier discoverable for reviewers with some time to spare. It also makes it easy for marvin-mk2 to automatically triage them. The plan is to regularly

• Search for (3), (6) sorted by least recently updated. Timeout those that are stale for let's say >=3d.
• Search for (5) sorted by oldest (since least recently updated could create perverse incentives). Assign privileged reviewers as long as available (i.e. people who have signed up as a reviewer and whose rate limit has not yet been reached)
• Search for (2) sorted by oldest (since least recently updated could create perverse incentives). Assign privileged or unprivileged reviewers as long as available.

Also note that only a single transition (the green one) needs manual intervention. People seem (understandably) a bit hesitant to spam PRs with /status messages, so this should be an improvement. Manual changes should still be possible if the heuristics fail.

Regarding naming of the labels, I'm thinking
(1) -> unlabeled / work_in_progress
(2) -> needs_reviewer
(3) -> awaiting_reviewer
(4) -> awaiting_changes
(5) -> needs_merger
(6) -> awaiting_merger

[timokau]

A note on why needs_review is not enough: Unfortunately github search doesn't allow you to restrict the search to PRs whose last update was by the PR author. Otherwise searching for PRs who have been updated by their author a couple of days ago could be used to simulate the "Needs new reviewer" state. Having an explicit label for that state makes it easier to search for anyway.

[ryantm]

I think you have the naming of labels 3 and 4 swapped.

[timokau]

Right, thanks for catching. Should be fixed now.

[ryantm]

Maybe "merger" could be "final_reviewer".

The author would know that it is close to done, but might need more review, and regular reviewers would know a "final" review is special.

[timokau]

Mh, "final" has a bit of the same problem as "merger (seems to suggest that the PR author shouldn't expect any additional rounds of review). At the same time it is a bit longer, so I think I prefer "merger" among the two.

Thanks for the idea though :)

[turion]

This is getting quite complex.

[timokau]

Yes, but also a lot more useful. Its a tradeoff. I don't know of a good way of supporting

• automatic reviewer assignment (making sure no PR gets drowned out in the queue),
• merger assignment (making sure reviews of non-committers mean something),
• easy discovery of PRs that need attention and
• automatic triage (making sure no PR gets lost due to inactive reviewers).

without a more complicated state graph. Feel free to suggest simpler alternatives.

[timokau]

Implemented in #30 and already deployed.

[blaggacao]

Maybe #40 can simplify.