Page MenuHomePhabricator

Patches with Verified -1 should not be counted as open in our code review metrics
Closed, ResolvedPublic

Description

In order to support T102920: What to do with old open patches for unmaintained/inactive repositories when not even the uploader responds, patches with Verified -1 should not be counted in our code review metrics, regardless of their CR value.

Just wondering, is there a Verified -2 value?

Related Objects

StatusAssignedTask
DuplicateQgil
ResolvedQgil
ResolvedQgil
InvalidNone
InvalidNone
OpenNone
DeclinedNone
ResolvedQgil
ResolvedQgil
ResolvedQgil
ResolvedQgil
ResolvedNone
ResolvedAklapper
ResolvedDicortazar
ResolvedQgil
ResolvedAklapper
OpenNone
ResolvedQgil
ResolvedAklapper
ResolvedAklapper
ResolvedDicortazar
ResolvedAklapper
ResolvedDicortazar
Resolvedmmodell
ResolvedLegoktm
Resolvedtstarling
Resolvedgreg
ResolvedAklapper
ResolvedAklapper
ResolvedAklapper
ResolvedAklapper
ResolvedNone
DeclinedNone

Event Timeline

Qgil created this task.Aug 9 2015, 9:05 AM
Qgil raised the priority of this task from to Low.
Qgil updated the task description. (Show Details)
Qgil added a subscriber: Qgil.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 9 2015, 9:05 AM

Just wondering, is there a Verified -2 value?

Depends on each repo's configuration but no "verified: -2" exists in our Zuul configuration currently.

Aklapper raised the priority of this task from Low to Normal.Aug 28 2015, 9:52 AM

@Qgil clarified "In the same way we filter CR-1; Jenkins can vote in a different and automatic way so we have some patchsets that are e.g. CR-0 but V-1". So these patches are not waiting for a reviewer but they are waiting for an author/submitter to update/rebase.

@Dicortazar said "shouldn't be too hard because it's a global filter but we should have a look, because of KPIs in another path in the library".
@Dicortazar to check if such changesets are currently accounted for author/submitter.

Aklapper set Security to None.

Wondering if having that Verified -1 in mind, the definition of 'open' is the definition of 'patchsets waiting for a submitter action'.

The question is: do you want to mark as 'open' only the changesets waiting for a submitter action? (so they were either reviewed or negatively verified).

Qgil added a comment.Sep 14 2015, 1:12 PM

At http://korma.wmflabs.org/browser/gerrit_review_queue.html we have "New" and "Waiting for review", and then "Time from last patchset" which is calculated from the 'waitig for review'.

Verified -1 would still count for "New", but not for "Waiting for review", and therefore not for "Time from last patchset".

I guess we've worked on that in the past. It seems and I think it is already fixed. Checking the code in GrimoireLib:

Any evidence this is failing or something?

According to comments by @jgbarah and @Acs, this ticket is not a bug. Closing this.

Please, let us know if you see some inconsistencies.

Dicortazar closed this task as Resolved.Oct 19 2015, 12:50 PM

Ah, thanks!

Looks to me like you're right from a quick test: I tried with "Gadgets" on http://korma.wmflabs.org/browser/gerrit_review_queue.html?page=22 listed as zero items waiting for review. That repo has one open patch in total in Gerrit and that one patch is V=-1 and CR=0 and from Oct 9.