[Engineering] [Wikitech-l] Gerrit now automatically adds reviewers

Fri Jan 18 08:46:45 UTC 2019

> […] automatically add reviewers to your changes based on who previously has committed changes to the file.

I'm already overwhelmed with review requests. I'm also one of the
latest contributors in sooo many files that I'm worried the plugin
will add me to dozens per day from now on. This surprising addition
makes me worry very much about my sanity and the usability of my inbox
and Gerrit dashboards. Please, please, tune it down heavily.

Until now, I had a process to find reviewers:

1. For planned changes, it's already obvious who needs to do the
review: my team members. Often they don't even need to be added as
reviewers, or don't want to, but use the "review" column on our
Phabricator board. Automatically adding random *other* people is not
only useless in this situation, it's counterproductive and frustrating
for everybody involved. Other people should not waste their time with
patches like this. When they do, it's frustrating for the one who was
supposed to do the review, as well as for the "auto-reviewer". His
review is not helpful and ultimately not appreciated.

2. For code cleanups in core and codebases my team does not own I open
the list of merged patches on Gerrit to see if I can tell the names of
one or two main contributors. Often, the list will contain nothing but
the Translatewiki bot and a few people doing cross-codebase
maintainance work. These highly active people should *not* be the
first pick as potential reviewers for multiple reasons. Most
importantly, just because someone is very productive it's not ok to
expect him to accept even more workload. This is
super-counterproductive and ultimately leads to people burning out.
Secondly, just because someone updated, let's say, a call to a
deprecated core function does not mean he is familiar with a codebase.

3. I look at the files my patch happens to touch in my PHPStorm IDE,
enable the blame column that shows who touched a line last, and see if
I can find the one who introduced the code I'm touching.

All steps in this process involve *reading* the commit messages and
considering what people did, when and why. This can't be automated.

I do not entirely oppose the idea of adding reviewers automatically,
as long as I (as a reviewer) have a chance to easily tell the
difference between being added manually vs. automatically. For my
sanity, I will most probably setup an email filter that auto-deletes
all automated requests, and only look at these auto-reviews once a
week via my Gerrit dashboard.

Based on all these arguments this is what I, personally, find acceptable:
* Make sure no emails are sent for being automatically added, or make
sure it's possible to turn them off (without also killing the wanted
emails about manually being added).
* Make sure tool accounts like the library-upgrader or Translatewiki
bot are never added as reviewers.
* Never automatically add reviewers if there are other reviewers
already. Most importantly, if people have been added via the reviewer
bot already, that's more than enough.
* Only add 2 reviewers. 2 people will more likely feel like a team. 3
people are much more likely to all think "the other 2 will do it" and
all ignore it.
* Don't just pick the "most recent" contributor. That's most certainly
not the person you want (probably one who fixed a typo, or updated a
library). Implement an algorithm that can either understand who
touched which places in the code, and assigns them to patches that
happen to touch the same place again. Or go for an algorithm that (for
example) analyzes the last year and picks the 2 people who did the
most changes to a file in that time span.

If more than one of these criteria is not met or not possible, the
only solution I see is to make the plugin opt-in per codebase, not
opt-out as of now (because you can't expect me to opt-out from
literally a thousand codebases).

Thank you for keeping my inbox sane.

Best
Thiemo