We do have a bot that adds reviewers based on some regexes on https://www.mediawiki.org/wiki/Git/Reviewers

Code is at https://github.com/valhallasw/gerrit-reviewer-bot

I think @Dzahn made a task about doing something like this using heuristics - who has edited this file (significantly, or many times) in the history...

@Reedy: Yes, that script is a great help. My most recent commit to core commit https://gerrit.wikimedia.org/r/#/c/193506/ was merged in about two hours.
Is there a chance to extend that script for WMF deployed extensions, and would that make sense?
Or is the extension math https://gerrit.wikimedia.org/r/#/q/status:open+project:mediawiki/extensions/Math,n,z is a special problem that needs special treatment indepenedt of the regular code review discussion?

In T91190#1076450, @Physikerwelt wrote:

@Reedy: Yes, that script is a great help. My most recent commit to core commit https://gerrit.wikimedia.org/r/#/c/193506/ was merged in about two hours.
Is there a chance to extend that script for WMF deployed extensions, and would that make sense?
Or is the extension math https://gerrit.wikimedia.org/r/#/q/status:open+project:mediawiki/extensions/Math,n,z is a special problem that needs special treatment indepenedt of the regular code review discussion?

You can at least add yourself to https://www.mediawiki.org/wiki/Git/Reviewers#mediawiki.2Fextensions.2FMath (and you probably should) :)

In T91190#1076438, @Reedy wrote:

We do have a bot that adds reviewers based on some regexes on https://www.mediawiki.org/wiki/Git/Reviewers

You can also ask to be notified of every change of a repo in Gerrit preferences, which is functionally the same as being added as a reviewer to every changeset of that repo (except that it is not visible to someone looking at the changesets).

@Qgil, what are you proposing as the action here? We can't just add all project owners to review every patch.

Especially as they probably already watch patches for their projects.

In T91190#1076557, @Tgr wrote:

In T91190#1076438, @Reedy wrote:

We do have a bot that adds reviewers based on some regexes on https://www.mediawiki.org/wiki/Git/Reviewers

You can also ask to be notified of every change of a repo in Gerrit preferences, which is functionally the same as being added as a reviewer to every changeset of that repo (except that it is not visible to someone looking at the changesets).

Yes, except that you're not *supposed* to review them. I find it a much more discreet alternative.

In T91190#1076438, @Reedy wrote:

We do have a bot that adds reviewers based on some regexes on https://www.mediawiki.org/wiki/Git/Reviewers

Code is at https://github.com/valhallasw/gerrit-reviewer-bot

I think @Dzahn made a task about doing something like this using heuristics - who has edited this file (significantly, or many times) in the history...

What Reedy said, we have all the logic already in the form of a bot. Maybe readvertise that barely known bot on wikitech-l and wmf engineering lists.

In T91190#1076438, @Reedy wrote:

We do have a bot that adds reviewers based on some regexes on https://www.mediawiki.org/wiki/Git/Reviewers

Ok, this is good to know. Still, I wonder how spread and up to dat is that information. Could the bot be synced with the Gerrit data directly, to assure that any individual owner of any repo is added as reviewer to new patches uploaded ?

And still the question, is Differential capable of doing this based on the concept of repository owners? https://phab-01.wmflabs.org/owners/

In T91190#1098235, @Qgil wrote:

https://phab-01.wmflabs.org/owners/

It would be interesting to see this information. It's restricted. I would be courious to see who owns the Math extension.

https://phab-01.wmflabs.org/owners/ is empty. I should have searched better and use https://secure.phabricator.com/book/phabricator/article/owners/ instead.

In Gerrit the Math extension is basically owned by... nobody?

https://gerrit.wikimedia.org/r/#/admin/projects/mediawiki/extensions/Math,access

Rights Inherit From: mediawiki/extensions

which says

Rights Inherit From: [[ Rights Inherit From: mediawiki | mediawiki ]]

which says

Rights Inherit From: All-Projects

which doesn't name a single person specifically. :( A code contributor would need to find previous changesets and see who merged them in order to add reviewers manually...

Also, I found this old open task upstream:

T1258: Add an "Automatically CC Owners on Differential Revisions" checkbox to Owners

In T91190#1098235, @Qgil wrote:

In T91190#1076438, @Reedy wrote:

We do have a bot that adds reviewers based on some regexes on https://www.mediawiki.org/wiki/Git/Reviewers

Ok, this is good to know. Still, I wonder how spread and up to dat is that information. Could the bot be synced with the Gerrit data directly, to assure that any individual owner of any repo is added as reviewer to new patches uploaded ?

Wait, what? You want all owners of a repository to be added as reviewers to all new patches? Or do you mean just single-owner repositories?

In T91190#1098332, @Krenair wrote:

Wait, what? You want all owners of a repository to be added as reviewers to all new patches? Or do you mean just single-owner repositories?

I want that "When uploading a new patch, reviewers should be added automatically". The goal being that every new patch uploaded gets a first human response within a few days, so uploaders know what to expect in that review.

Having a bunch of reviewers added is basically the same or worse than having no reviewers. I guess I mean "individual reviewers", not those that can merge patches by inheritance of group ownership. And I guess I'm focusing on the small / not very active projects, because the big / active are at least watched by more developers. MediaWiki core deserves probably a pln on its own.

In T91190#1098324, @Qgil wrote:

In Gerrit the Math extension is basically owned by... nobody?

https://gerrit.wikimedia.org/r/#/admin/projects/mediawiki/extensions/Math,access

Rights Inherit From: mediawiki/extensions

which says

Rights Inherit From: [[ Rights Inherit From: mediawiki | mediawiki ]]

which says

Rights Inherit From: All-Projects

which doesn't name a single person specifically. :( A code contributor would need to find previous changesets and see who merged them in order to add reviewers manually...

I wouldn't be surprised if several more of these cases come up when you consider ldap/wmf (to get a list of these people you can run "ldaplist -l group wmf" on a labs or production host, but you get uid back rather than cn/mail)

@Qgil: I really apprechiate your efforts to help Math, but for the Math Extension it's not a technical problem.
On a management level it has been decided that there is no wmf developer time for the Math extension.
However, code review for the Math extension requires a high level of expertise i.e. with regard to the effects to the performance and security.
Until this managment decision is not revised, code review will happen, whenever a skilled developer finds time for code review in addition to her/his regular job.
I was considering to organizing an effort to create a politicall movement for better math support from the german chapter... but I had to realize that this was not the right time.
I'm convinced that Math support is important for Wikipedia and thus needs to be maintained in the same way as MediaWiki core is maintained, which requires developer time. My personal oppinion that this should be priotized over to new feature development. But again this is a political and not a technical decision and does not really belong to this thead. That said, the Math extension might not be the best example for the proposed change.

In T91190#1098235, @Qgil wrote:

In T91190#1076438, @Reedy wrote:

We do have a bot that adds reviewers based on some regexes on https://www.mediawiki.org/wiki/Git/Reviewers

Ok, this is good to know. Still, I wonder how spread and up to dat is that information. Could the bot be synced with the Gerrit data directly, to assure that any individual owner of any repo is added as reviewer to new patches uploaded ?

[...]

Please provide data on individual owners of any repository not being informed about incoming patches.

Ok, so you all seem to be saying that the problem is not lack of
notifications when new patches are uploaded for review. Should we then
decline this task and look elsewhere for solutions to the current situation?

An easy(?) thing to do is to check whether the review time of a patch correlates with the number of reviewers added to it.

In T91190#1098476, @Qgil wrote:

Ok, so you all seem to be saying that the problem is not lack of
notifications when new patches are uploaded for review.

I think it is, indirectly. People do get notifications (email from gerrit) when they are added as reviewers, but they also get ALL these other mails from gerrit, and i think what happens is that most people filter gerrit mail. Imho, smarter filters that don't discard ALL gerrit mail and let through the one that are actual review requests would be the way to go.

That, or forget about email and it's a social problem that everybody has to get used to regularly checking the web ui and look at their personal queue without needing extra notifications.

@Dzahn wrote:

People do get notifications (email from gerrit) when they are added as reviewers, but they also get ALL these other mails from gerrit, and i think what happens is that most people filter gerrit mail. Imho, smarter filters that don't discard ALL gerrit mail

I do read all my Gerrit emails, but I am an exception. People can filter / mark important mails that have the email header Gerrit-MessageType: newchange. It seems most people just filter them out and instead use the Gerrit homepage which is not proposing an helpful view (the default searches offered are not suitable for doing review).

Back on topic.

Imho there is no point in keeping this Task opened. Sam exposed in a previous comment T91190#1076438 how reviewers can craft rules that would add them as reviewers. Seems to me that fulfill this task request.

People not reviewing the changes, lack of triage, discarding the Gerrit notifications, absence of a wall of shame... are entirely different topics.

As a quick note, I have an idea to improve the adding of reviewers even without specific rules, by using the file history, a la https://blogs.atlassian.com/2014/07/git-guilt-blame-code-review/

I haven't had time to look at it, but if you're interested, we can take a look during the hackathon.

In T91190#1139450, @valhallasw wrote:

As a quick note, I have an idea to improve the adding of reviewers even without specific rules, by using the file history, a la https://blogs.atlassian.com/2014/07/git-guilt-blame-code-review/

I haven't had time to look at it, but if you're interested, we can take a look during the hackathon.

Less intelligent but off-the-shelf: reviewers-by-blame is a gerrit plugin that does a git blame on the parent patch and adds those who changed many of the lines which are also changed by the current patch.

Beside git blame, another trick I am using is fetching reviews votes from Gerrit as git notes and then display them in git log:

git fetch -v gerrit refs/notes/review:refs/notes/review
git log --notes=review

An example from a random commit in mediawiki/core:

$ git show --notes=review --oneline --no-patch 0c04667ca
0c04667 registration: Don't hardcode list of extension types in schema
Notes (review):
    Verified+2: jenkins-bot
    Code-Review+2: Alex Monk <krenair@gmail.com>
    Code-Review+2: Legoktm <legoktm.wikipedia@gmail.com>
    Code-Review+1: Paladox <thomasmulhall410@yahoo.com>
    Submitted-by: jenkins-bot
    Submitted-at: Sat, 30 May 2015 00:44:48 +0000
    Reviewed-on: https://gerrit.wikimedia.org/r/214778
    Project: mediawiki/core
    Branch: refs/heads/master

Authors of those Code-Review votes are potential reviewers.

@Tgr maybe we can get that Gerrit reviewers-by-blame plugin. A nice thing is that it can be enabled on a per project basis. Worth filling a task for #wikimedia-git-or-gerrit.

In T91190#1328331, @hashar wrote:

@Tgr maybe we can get that Gerrit reviewers-by-blame plugin. A nice thing is that it can be enabled on a per project basis. Worth filling a task for #wikimedia-git-or-gerrit.

T101131

I had the task closed since we have https://www.mediawiki.org/wiki/Git/Reviewers

Yesterday we have enabled a plugin in Gerrit which automatically add reviewers based on the code history T101131 . So yeah, that is done automatically now :] Albeit not perfect, that will surely help for T78768: Agree on and implement actions to prioritize code review of patches submitted by volunteers.

I have published a blog explaining the Gerrit review by blame plugin as well as other manual way to find reviewers: J139: Blog Post: Gerrit now automatically adds reviewers

Eventually the Gerrit plugin from T101131 has been disabled. Adding reviewers solely based on them having the last author does not reflect our reality. We might enhance it later on and consider deploying it again, meanwhile I think we can abandon the idea for now (eg keep this task in invalid status).