[teampractices] Code review before writing new code

[Quim Gil]

Er, sorry, the previous email just left my mailbox too fast.

The proposal below has a related Phabricator task:
https://phabricator.wikimedia.org/T101686

https://office.wikimedia.org/wiki/Health_check_survey_results/FY2014-15_Q3
indicates a growing concern about code review. Our code review queues keep
growing, I'd say still faster than our concerns, and this trend can only
lead to some variation of *collapse*.

http://korma.wmflabs.org/browser/gerrit_review_queue.html shows these
monthly snapshots of open chagesets waiting for review (-1 and work in
progress are not counted):

May 2013: 250
May 2014: 1033
May 2015: 1549

In order to stop this trend, revert it, and clean our queues we need a
radical change. I want to propose a basic principle that should guide our
software development practices:

"A team should review their open patchsets before writing new code."

This is not impossible, just a matter of setting priorities. Exceptions
could be made as long as their queue of contributions waiting for review is
diminishing. The average reviews should probably be harsher, not hesitating
to give a -1 and a short explanation that brings the initiative back to the
original uploader, or a -2 and another short explanation when patches are
just too bad or not compatible with the scope or roadmap. Any of these
measures would be better than an ever-growing queue of languishing patches.

This principle would also help highlighting deeper problems, mainly
repositories and whole areas of code that are unmaintained in practice.
Knowing these areas, we could decide to either allocate/empower new
maintainers, or just put a big banner indicating that "Sorry, this
repository doesn't welcome new contributions as we are not able to process
them adequately", or, again, something better than just let patches rot in
silence.



PS: I'm sending this email to team-practices and not wikitech-l because
this is a matter of team practices and team management in the first place;
pinging individual developers as some of us do in Gerrit sometimes is just
not enough.


On Mon, Jun 8, 2015 at 10:23 AM, Quim Gil <qgil at wikimedia.org> wrote:

> Thank you, very useful. I'm happy to see that our problem with code review
> keeps becoming more evident, if only as a step to finally doing something
> about it.
>
> http://korma.wmflabs.org/browser/gerrit_review_queue.html shows that our
> backlog of patches waiting for review keeps growing. 1542 last month, was
> 1033 a year ago. If you consider the time and skills it takes to produce
> one patch, you may imagine the big black hole of frustration and waste of
> real money we have there.
>
> While comparing satisfaction across teams is subjective and complex,
> comparing code review waiting times between WMF teams can be measured
> objectively. Engineering Community is aiming to organize a first Gerrit
> Cleanup Day [1] focusing on open patches submitted by volunteers, and we
> could work on these metrics for the second one -- unless someone wants to
> start earlier, of course.
>
> [1] https://phabricator.wikimedia.org/T88531
>
> --
Quim Gil
Engineering Community Manager @ Wikimedia Foundation
http://www.mediawiki.org/wiki/User:Qgil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.wikimedia.org/pipermail/teampractices/attachments/20150608/ce537d14/attachment.html>