Page MenuHomePhabricator

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

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).

Isarra added a subscriber: Isarra.Jul 27 2018, 1:16 PM
Aklapper updated the task description. (Show Details)Jul 27 2018, 1:55 PM

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?

xSavitar updated the task description. (Show Details)Nov 17 2018, 8:36 PM
xSavitar added a subscriber: xSavitar.
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
Aklapper added a comment.EditedApr 19 2020, 3:34 PM

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.

For my records, I asked Bitergia for support in (non-public) https://gitlab.com/Bitergia/c/Wikimedia/support/-/issues/80

Aklapper added a comment.EditedJun 7 2020, 6:06 PM

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:

...and a screenshot of the outcome:

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

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

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. :)

Aklapper updated the task description. (Show Details)Jul 28 2020, 4:36 PM
Tgr added a subscriber: Tgr.Aug 4 2020, 10:53 AM

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
Tgr awarded a token.Aug 4 2020, 11:00 AM