Last modified: 2014-06-06 15:10:17 UTC
In metrics about open Gerrit changesets like http://korma.wmflabs.org/browser/gerrit_review_queue.html , those changesets with -1 should be ignored, because from the point of view of the maintainers they are not in their ToDo list anymore. If those -1 are not touched, they will be eventually abandoned. This should be applied to all graphs about "pending submissions", "open changesets". Maybe we should apply a delay in order to avoid graphs changing radically just because a changeset can be pending on Monday, -1 on Tuesday, pending again on Wednesday... Let's start ignoring those that have been updated more than 30 days ago and have -1. Based on the graphs we get, we may want to fine tune this. > > > Does "open" include changesets that were submitted, got feedback, and then > > > the submitter never bothered to follow up? > > > > > > > Yes, shouldn't we? > > > > Open is open. What would be the best practice with these changesets? Just > > leave them open like now, abandon them, mark them with some tag...? > > > Best practise is for author to abandon them if they are not interested any > more. However very few people actually do that. If you want to exclude > them, -1 is a good criteria (-1 patchsets are not waiting on review, so > even if there is still interest, its not in queue). http://lists.wikimedia.org/pipermail/wikitech-l/2014-April/075646.html
Hi! In OpenStack projec, the policy is that a submissions with -1 and not touched in 1 week is moved automatically to ABANDONED status. Something like that make sense in Wikimedia also? A query to detect some of this submissions: select count(changes.id) as total, issue_id, new_value, status, changed_on from changes, issues where changes.issue_id = issues.id AND status = 'NEW' and (new_value="-1" or new_value="-2") and DATEDIFF(NOW(),changed_on) > 30 group by issue_id having total=2; A list of 143 submissions in this state are detected. We can move all of them to ABANDONED state but maybe, it is better to do it in Gerrit directly.
Hmmm, this query detect patchSet with -1 not touched but a submissions could have several patchSets. We should only take care that this corresponds to the last patchSet!
I have some changesets where I uploaded a reworked patchset several weeks or even months after an original -1 was given...
Two things. This isn't that trivial to implement in Metrics Grimoire. Alvaro can explain the details if needed, but basically what happens is that each upload needs to be checked to make sure that it is the latest of a changeset. The other thing, more important actually, is that perhaps the solution is to change our policy, and abandon -1 changesets after some period. There is a discussion ongoing at http://lists.wikimedia.org/pipermail/wikitech-l/2014-April/075825.html We will decide what to do once we know what happens with the policy.
Well ok, after a first wave of replies it looks clear (to me) that we are not going to implement an automatic process for abandoning changes anytime soon. Even if we do it, the margin of time could be e.g. three months, which brings back the problem of not reflecting the patches that are really waiting for a review. Therefore, Alvaro, your further investigation to see how can we fix our metrics without changing our current process is welcome. :)
Just in case it's not clear, what needs to be filtered are -1 and -2 (label:Code-Review<=-1). They've always been filtered in our lists of open commits: https://www.mediawiki.org/wiki/Gerrit/Reports#Commits_lists (query first shared by robla).
Alright, let's filter changesets with label:Code-Review<=-1. We can add the delay later one if we really need it.
A first version for getting comments: http://korma.wmflabs.org/browser/gerrit_review_queue2.html The repositories data is now ordered by pending reviews waiting for reviewer and this metric is shown (ReviewsWaitingForReviewer). Next step could be to modify the pending time (Update time for pending reviews waiting for reviewer in days), but I am not sure about it and I prefer to share with you this first approach and decide next steps.
This is useful, thank you. "Number of pending submissions" can keep both lines, it is useful to see the difference between the total amount of open reviews and those waiting for a reviewer. "Age of pending submissions" could also have blue/green columns. If only one value could be showcased, then I would rather go for the changesets waiting for reviewer. "Review time for open submissions..." and "Are Wikimedia's staff..." should be calculated based on ReviewsWaitingForReviewer only. In the list of repositories, ideally "pending" and "ReviewsWaitingForReviewer" would be merged with blue/green lines. If this is not possible, then we can keep them separate. "Update time for pending in days" would be calculated based on ReviewsWaitingForReviewer and would be the criteria for sorting the list. I think this approach solves the problem in a better way than I had initially proposed, thanks to your idea of bringing a prototype first.
Created attachment 15238 [details] Who is doing better and worse? Sorry, can't parse this.
Quim, all work done in: http://korma.wmflabs.org/browser/gerrit_review_queue2.html We have included both metrics, for all reviews pending, and only for reviews pending for reviewers. Once you check all, we will remove the all reviews pending and just leave the graphs with "for reviewer". We will wait for you comments for future steps!
Ok, updated all in: http://korma.wmflabs.org/browser/gerrit_review_queue.html after yesterday meeting.
Thank you! One graph that makes me wonder is "Are Wikimedia's staff and non-staff contributions processed equally?". How can all the values be 0 on April 2013, when in the graph above we see that the general median is 51,1 days?
I need to review it, probably, we are filtering the data before Apr 2013. There are troubles with data until May 2013, so this issue could be related to that.
Quim, after reviewing, the problem was the predicted: * For companies we are starting the analysis from startok = "'2013-04-30'" and this is why in April the values are 0. * For global data, the start time analysis is the start of the report, 2010. We have modified the global analysis so it also starts i "startok" and the data should be the same. Also, the start time for both graphs will be moved to May 2013 to remove the empty April 2013 data.
All fixed now in korma: http://korma.wmflabs.org/browser/gerrit_review_queue.html
Yep, thank you!
