We should define some sort of cut off for just abandoning commits (over some date, even more so if CR-1/CR-2 or V-1). They're just clutter. They still exist, and if someone wanted to work on them, they can be easily restored.

Probably with a nice summary, "thanks for the commit, you can restore this at a later date if you plan on working on it" etc

For example mw core has 29 commits untouched since 2013...

You could argue that untouched patches over a year old are candidates for abandoning etc... Core has around 100 commits that are a year old

Maybe it would help to generate list of commits to reviews per project/authors. That might attract people to do the cleanup.

For WMF, maybe it can be a special day at the office with people gathering around tables to do the cleanup and not be disturbed with other duties.

In T113378#1669853, @hashar wrote:

For WMF, maybe it can be a special day at the office with people gathering around tables to do the cleanup and not be disturbed with other duties.

"The site is down! Tough! I'm doing Code Review and Gerrit Cleanup!"

I've dumped my thoughts and impressions above.
Comments and discussion welcome.

As I said elsewhere: The problem isn't "management", but "reviews". Nobody is helped by the assurance that their patch might (!) be reviewed at a Review Day in a few months time where there is social pressure to process as many patches as possible, so that in the case of a (probable) negative review they might (!) get another chance in another few months time.

I don't find the posting to wikitech-l at the moment, but IIRC the mobile team had a policy to review each contribution to "their" repositories in x days. IMHO that's the way to go, no organization or travelling and cookies needed, but a simple script that alerts "maintainers" (to be defined) every week that they have y patches waiting for review.

As a volunteer, I didn't really have much idea how to get involved, though this seems like a good way, potentially, to get all manner of folks from the community working together on the same things as well. I tried asking in #wikitech-l during the thing, but nobody who seemed to know anything was around and then I just got distracted and nothing came of it at all.

Something to maybe consider, I dunno.

Mass comments like "-1 needs manual rebase" are NOT helpful unless it was the intention of the reviewer to merge the patch. The -1 discourages other reviewers, without actually checking the substance of the patch. And we know that gerrit's "Can't merge" indicator is often broken, there are plenty of times when actual git rebases the patch just fine.

In T113378#1666472, @Reedy wrote:

We should define some sort of cut off for just abandoning commits (over some date, even more so if CR-1/CR-2 or V-1). They're just clutter. They still exist, and if someone wanted to work on them, they can be easily restored.

Probably with a nice summary, "thanks for the commit, you can restore this at a later date if you plan on working on it" etc

For example mw core has 29 commits untouched since 2013...

You could argue that untouched patches over a year old are candidates for abandoning etc... Core has around 100 commits that are a year old

This is a standard practice in some open source projects, and I think it would be a good idea for us as well. We can start off incredibly generous, only abandoning patches that havn't been touched in a year. But other projects (such as HHVM) will close open patch sets with three weeks of inactivity. It's not like the patches are lost, abandon just means its not going anywhere. Anyone who wants to can restore the patch set.

In T113378#1672213, @EBernhardson wrote:

This is a standard practice in some open source projects, and I think it would be a good idea for us as well. We can start off incredibly generous, only abandoning patches that havn't been touched in a year. But other projects (such as HHVM) will close open patch sets with three weeks of inactivity. It's not like the patches are lost, abandon just means its not going anywhere. Anyone who wants to can restore the patch set.

We had past discussion about it http://www.gossamer-threads.com/lists/wiki/wikitech/450845 . Maybe we can fill a sub task as a follow up?

First of all, let me say thank you to @Aklapper and all the people involved in this experiment. You made it! I think you made a difference indeed, and we will continue to work cleaning up our code review queues.

The main conclusion we need to make is whether it is worth organizing a next Gerrit Cleanup Day. I think so. We made a dent on 17%-25% of patches submitted (presumably) by volunteers that hadn't received any review. It's not the goal we had set (slightly blindly, but well) but it is a remarkable number. Thanks to this Day we created Gerrit queries that continue to be useful, and we raised the awareness on this problem. This and other related actions we are taking should help keeping healthier numbers, and hopefully should help also organizing a more successful second Gerrit Cleanup Day.

On promotion and following of the Day, do we have any idea about the participation of WMF teams? Did everybody know? Was everybody involved? Were there people that knew but didn't know how to help? Was someone ignoring this action, writing / committing code that was not urgent to deploy? Not to go after people, but to see what can we as organizers do better next time.

And the other way around, do we have a way to highlight and praise the most effective developers / repositories / projects / teams during the Day?

Was the activity counterproductive in any way, for instance promoting unproductive -1 reviews as @Legoktm points out? Can we prevent this?

Also following on @Isarra's comment, how can we improve involving volunteers, also those that might realize on IRC etc that this activity is going on right now?

We should also recover the comment made my Mark somewhere, that we should not organize the next Day so close to the end of the quarter, when most of us are running to match deadlines and working on extra documentation, a situation that is more rare during the rest of the quarter.

PS: fresh tasks T113706: Define Cut-off date for abandoning old changesets in our code review tool and T113707: Decision on Wikimedia Foundation service-level agreement on code review times.

There was a lot of churn in WMF codebases. Although one of the explicit goals of the cleanup day was *not* to hastily merge patches, but instead to offer good reviews, I think we should pay next attention to the next Mediawiki train deploy (week of Sep 28) to see whether this cleanup day set more wikis ablaze than usual.

If so, I'd suggest that we try to accommodate the cleanup day in the deploy schedule somehow next time. Perhaps skip the group2 deploy for the week following and let the code live on the group1 servers for an extra week to get hammered on, or something like that.

But if deploy goes swimmingly, then maybe this extra concern is misplaced. We shall see next week!

(Alternatively, if this becomes a more regular activity, one might hope that *not quite as many old patches* will need to be reviewed/merged. That would also reduce deploy risk.)

@cscott the Gerrit clean up day has been taken in account by Release-Engineering-Team and extra care is taken for the deployment train that follow. We definitely want to keep it in mind for the next cleanup day for sure.

The Discovery department (finally) did a quick retrospective on gerrit cleanup day. I'm not sure how to integrate these notes into the categories in the main body of this phab ticket:

• Not many pending patches for Discovery, so there wasn't much direct benefit to us
• (Some developers) did good work in core; it was good for the org overall
• It was unclear how to prioritize our team vs. backlog items in general
• Still getting a couple emails a week on issues raised during cleanup day (which is a good thing)
• (Some developers) learned about other parts of the codebase
• Not much for data analysts to do (unclear whether more communication across departments would have helped)
• Tangential takeaway: We should be interacting more with the community

There is a proposed WikiDev16 session about improving code review, especially for volunteers: T114419: Event on "Make code review not suck". You are welcome to comment there if interested.

In T113378#1671663, @Legoktm wrote:

Mass comments like "-1 needs manual rebase" are NOT helpful unless it was the intention of the reviewer to merge the patch. The -1 discourages other reviewers, without actually checking the substance of the patch. And we know that gerrit's "Can't merge" indicator is often broken, there are plenty of times when actual git rebases the patch just fine.

In the Wikidata team we moved to marking these changes as V-1.

I just learned about this bug from the Developer Relations Weekly Summary (thanks @Qgil) and I would like to say big thank you to everybody involved. Some of my oldest stuff got suddenly rebased by a group of reviewers, I was really surprised.

Frankly I thought this is some kind of a weekly cleanup action (a bit sad to hear it was a one-time event for now), but from my perspective the experience was very positive.