Page MenuHomePhabricator

Allow to filter by Gerrit label 'code review' in the frontend
Open, MediumPublic0 Estimate Story Points

Description

Implemented in the backend via https://github.com/chaoss/grimoirelab-perceval/issues/357
Not implemented in the frontend via https://github.com/chaoss/grimoirelab/issues/96

Non-public internal ticket: https://gitlab.com/Bitergia/c/Wikimedia/support/issues/61

Implementation as of December 2019

On any Gerrit dashboards on https://wikimedia.biterg.io,

  • set status:"NEW" AND status_value:"1" to get data about open patches with the CR+1 label
  • set status:"NEW" AND status_value:"-1" to get data about open patches with the CR-1 label
  • set status:"NEW" AND !_exists_:status_value to get data about open patches without a CR label (CR=0)

Event Timeline

Probably means splitting NEW in the screenshot below into Open && CR0 (Unreviewed), Open && CR+1, Open && CR-1, Open && CR-2, and maybe Open && CR+2. Also wondering if the separate Verified label should be ignored or not, because you may not want to review a CR0 && V-2 patch?

For the records, in the backend, Perceval is currently retrieving that field in the approvals array for a Gerrit change under value, as Kibana's "Discover" shows.

Aklapper raised the priority of this task from Low to Medium.Jun 20 2019, 8:38 AM
Aklapper renamed this task from Provide Gerrit status values (labels) for patchsets in the frontend to Allow to filter by Gerrit label 'code review' in the frontend.Aug 5 2019, 8:55 PM
Aklapper updated the task description. (Show Details)

There is a patch at https://github.com/chaoss/grimoirelab-elk/pull/679 :

In a nutshell, for each changeset, the last approval value that meets the conditions below is retrieved and added to the enriched item as status_value.

  • the approval is not of type Verified
  • the patchset author must be different from the approver

Reporting progress on the backend here:
Looking at https://wikimedia.biterg.io/goto/326470f410eaa0a5fa1808e56ad037ac (url:"https://gerrit.wikimedia.org/r/533713"), comparing with https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Scribunto/+/158323/ one can see the approval_value changing over time:

Now to sort out having this in the frontend and being able to filter on it; wondering if there is a better approach than T224755#5227113.

Quoting Bitergia folks:

[...] there will be a new field status_value showing the latest status of each changeset. From that, you could do such splitting by filtering in NEW changesets and then focusing on status_value.
It is also possible to specify filters at bucket level, so you can then define things like:

  • NEW (+1)
  • NEW (-1)
  • Merged
  • ...
Aklapper moved this task from Backlog to Doing on the wikimedia.biterg.io board.Dec 3 2019, 2:59 PM

On the Gerrit dashboards, it is now possible to filter on the status_value field (which resembles the Gerrit CR label). Examples are:
status_value:"1" AND status:"NEW" or status_value:"-1" AND status:"NEW". However, still trying to find out how to query for CR=0. Stay tuned.

Alright, after playing a bit with the Gerrit index in "Discover" plus rtfm, status:"NEW" AND !_exists_:status_value seems to resemble CR=0.

Aklapper updated the task description. (Show Details)Dec 14 2019, 8:54 PM

Pretty much done, only keeping this open because of a small data inconsistency that I don't understand yet.

only keeping this open because of a small data inconsistency that I don't understand yet.

Quoting non-public conversation (with some minor editing by myself, to understand better):

What I did on 2019-12-17:

Reply by Bitergia in https://gitlab.com/Bitergia/c/Wikimedia/support/issues/61 :

if you look at https://wikimedia.biterg.io/goto/30abf05cefb0ae682bbcd4cc1775c080 you can see that the latest code review for PS19 is -1 (and PS21 has no review yet as of 2019-12-17). The rest of +2 are not code reviews but bot verifications. In fact, we ignore these in the dashboard as they are not code reviews.
If you go to Approvals dashboard (https://wikimedia.biterg.io/goto/32751e02e11dabd05b85c3289bdeb73c) you'll see there are no +2 reviews unless you disable bots filter. That's why the current status for this changeset is -1.

What I replied on 2019-12-20:

Indeed CR-1 was the value set for previous Patch Set 19 (and the last CR value ever set), and the latest Patch Set 21 has no CR value yet on https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Flow/+/477989/ :
So I guess my question is: How to phrase a search query to list all changesets whose latest Patch Set does not have a CR value yet? Asking as I am after identifying changesets whose latest Patch Set is awaiting a code review.

Reply by Bitergia in https://gitlab.com/Bitergia/c/Wikimedia/support/issues/61 :

I'm not sure if this can be done. We face a similar problem when looking for the time to first review:

Time to first review data is already available in the enriched index (time_to_first_review for changeset and patchset_time_to_first_review for patchsets).
Note that in some cases the approval date is earlier than the creation date of the changeset/patchset. This happens because there is no approval after a new patchset. The previous approvals are copied to the new patchset, although they come from the previous patchset (this is not something we do but something that comes from the server). To overcome these cases and avoid negative numbers, we skip approval dates previous to patchset date to calculate the time to first review.

We could look for -patchset_time_to_first_review:* AND type:patchset and that should give us the patchsets without later review (not 100% sure as it gets trickier on each step :P). Something like: https://wikimedia.biterg.io/goto/dd92a4912a3c1c4cecb81dca3855ba1b
Please review this and if you still not find what you need, open a new issue when this specific use case so we can have a thread dedicated to it (in case we need to make any modification in the index to support it)

...and putting my notes from an impromptu meeting with Bitergia at FOSDEM 2020 here, so I don't need to search them again:

Discussed https://gitlab.com/Bitergia/c/Wikimedia/support/issues/61 - Gerrit API returns somehow CR label for first patchset when querying for second patchset - time is from first patchset to actual review. Andre would like to find/filter on latest patchsets without review while previous versions had a review. Maybe to filter on patchsets without any value? Two possible approaches here. Andre to define a better use case. https://phabricator.wikimedia.org/T224755#5813556