Page MenuHomePhabricator

Develop a workflow to recommend / propose developers who should have +2 rights in Gerrit
Open, LowestPublic

Assigned To
None
Authored By
Aklapper
Jul 11 2018, 10:04 PM
Referenced Files
F31942552: a.png
Jul 21 2020, 1:41 AM
F31942550: b.png
Jul 21 2020, 1:41 AM
F31857311: t196-02.jpg
Jun 7 2020, 6:06 PM
F31857312: t196-01.jpg
Jun 7 2020, 6:06 PM
Tokens
"Yellow Medal" token, awarded by Tgr.

Description

Capacity building.
Making a final +2 decision can be one of the bottlenecks in our code review process. (Docs about granting: https://www.mediawiki.org/wiki/Gerrit/Privilege_policy )

https://lists.wikimedia.org/pipermail/wikitech-l/2018-June/090222.html brought this to my mind again though I had already listed it under https://www.mediawiki.org/wiki/User:AKlapper_(WMF)/Code_Review#Lack_of_enough_skillful,_available,_confident_reviewers_and_mergers

Event Timeline

:)

I have some more thoughts that I'll post later but for now:

  • FInd out if we can use the list of most active contributors on https://wikimedi.biterg.io and how to best check which repositories they are the most active in

I've been using a search similar to https://gerrit.wikimedia.org/r/q/status:merged+project:mediawiki%252Fcore+-ownerin:mediawiki+-owner:L10n-bot+-ownerin:ldap%252Fwmf (h/t to Krenair for the search to replicate the filtering that I was doing manually) to identify potential +2 candidates.

The mediawiki group grants +2 to any repository under mediawiki/ in Gerrit, as well as some PHP libraries that are used by MediaWiki but predate the mediawiki/libs/ naming convention. There are also the individual extension-Foo groups that grant +2 for for that repository, and some other team/org based groups that are in the ACLs for related projects (e.g. Wikidata, ShoutWiki, the old editor-engagement group).

High level, simplified:

  • Get a list of people who are the most active for each repository
  • Get a list of people who have +2 for each repository
  • Diff.

To get a list of people who have +2 per repository, I don't see any way via https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html (just getting a list of repositories for a specific user, which is the other way round). Any ideas?

Aklapper renamed this task from Develop a workflow to propose handing out +2 rights in Gerrit to Develop a workflow to recommend / propose developers who should have +2 rights in Gerrit.Aug 22 2019, 10:34 AM

Nowadays we have a "Gerrit Approvals" dashboard at https://wikimedia.biterg.io/app/kibana#/dashboard/95487340-6762-11e9-a198-67126215b112 and it has visualizations like Approvals by Reviewer (across repos), Approvals by Reviewer, Project and Repo (both on the field author_name), and Approvals by Submitter (on the field changeset_author_name).
Each metric value in the tables is a Sum Bucket Aggregation: Overall Sum of Count with the Sub Aggregation = Filters and Filter 1 set to approval_value:"X" (X being -2, -1, 1 or 2).

For the first two visualizations I wonder if Query DSL allows to filter on only showing author_names whose aggregation for approval_value:"2" is 0.
If anyone knows Elasticsearch and has a hint, it's welcome.

(For the Approvals by Submitter visualization (patch author) I wondered how to "exclude" patch authors who have +2 in some repo but I don't see how that is possible, plus the table is cross-repo anyway hence not feasible.)

Also still clueless how to efficiently find out which person has +2 in which repo(s). Somehow.

There is an experimental custom dashboard at https://wikimedia.biterg.io/app/kibana#/visualize/edit/a023d580-8eb1-11ea-8a35-09ce380aa6b8 .
For everyone to better understand how that dashboard was created by Bitergia, adding a screenshot with its settings:

t196-01.jpg (1×840 px, 179 KB)

...and a screenshot of the outcome:

t196-02.jpg (514×679 px, 54 KB)

Could apply author_bot:false to get rid of the SonarQubeBot entry which is a bot. (For the records, SonarQubeBot has been changed to use the Verified label instead of the CR label in the meantime, but still remains in this list as it votes on Verified with the value +1 and not a +2 value.)

All in all, still not too helpful IMHO, as this is across repositories. :)

Aklapper triaged this task as Lowest priority.Jul 21 2020, 1:41 AM

Quoting internal comment:

AFAIK the only way is using/editing a visualization. We need to calculate that value using a sum aggregation so that's why you cannot use a filter

b.png (307×304 px, 14 KB)

Ideally I'd love to completely exclude all reviewers in that list whose value for CR+2 > 0, not only "sort them at the end".

You can filter directly the visualization if you go to Options and select the column and the value in the field Lines computed filter plus you can hide columns like CR+2

a.png (306×328 px, 13 KB)

Data itself is already available by going to the default dashboard https://wikimedia.biterg.io/app/kibana#/dashboard/95487340-6762-11e9-a198-67126215b112 and using the Approvals by Reviewer widget and sorting by the 2 column (means: CR+2 in Gerrit) to sort by reviewers who the value 0 in that 2 column, but I'm unable to tell some web browsers to use the Approvals column as the second column sort criterion. Means: not very usable.

Hence created a custom widget C_gerrit_reviewers_CR1_without_CR2 at (thanks to Bitergia for explaining computed columns, see previous comment here) which is also cleaner.
Widgets need to be on dashboards, hence created a dashboard at:

https://wikimedia.biterg.io/app/kibana#/dashboard/ffd01840-cdf9-11ea-8358-4d35848e335d

Also added the gerrit_approvals_by_code_reviewer_project_and_repo widget, so once you filter on a reviewer, you can see a list of the repos in which the patches are which they reviewed. However that loads really slow as that widget always has to load all data for all and any reviewers and repos (plus no restriction to reviewers with zero CR+2 actions either) for the defined timeframe. Garr. But patience: Works.
And I have no idea however to also create a widget which will list the URLs and summary for those actual Gerrit patches that the selected reviewer +1'ed.

Still: Shows me that Ammarpad has made 127 code reviews in the last 6 months, 48 of them in MediaWiki Core. Nice.

Added documentation to https://www.mediawiki.org/wiki/Community_metrics#List_of_the_most_active_reviewers_without_CR+2_actions

HTH - was at least an interesting trip for me into dashboard lands on a Friday evening. :)

Gerrit queries that tend to be useful:

  • Comments from the user on patchsets created by others: https://gerrit.wikimedia.org/r/q/commentby:%2522<user>%2522+-owner:%2522<user>%2522
  • +1 (non-self) reviews from that user: https://gerrit.wikimedia.org/r/q/label:Code-Review%253D%252B1%252Cuser%253D<user>+-owner:%2522<user>%2522
  • -1 (non-self) reviews from that user: https://gerrit.wikimedia.org/r/q/label:Code-Review%253D-1%252Cuser%253D<user>+-owner:%2522<user>%2522

I first want to see T273164 sorted out, so I'm punting this by a quarter (though I'm still not convinced that data allows us to have a really good insight).