How I started reviewing patches faster

The discussion about whether and how we should do patch reviews faster has been brought up for a few years now.  Here is the story of how I decided to stop being lazy start to actually review patches faster.  It’s been working very well from me and from people who ask me for review for more than two years now, and I hope that it helps other reviewers at Mozilla as well.

My typical day usually starts by looking over my email.  Every day at 9:00am local time, if I have unhandled review requests, I get an email with the subject line “Pending requests from me.”  I have a Bugzilla whine set up for a saved search like this.  Bugzilla sends out an email to me if there are any search result at that time every day, with a list of those bugs.  This helps me quickly go through all of those patches.

For every patch that is pending my review, I first go through the patch very quickly and decide whether I’m the right person for this review.  If I’m not, I can usually think of a better person and I will redirect that request towards them.  Otherwise, I clear the review request and comment on the bug suggesting that another person should review this patch.  If I am the right person, I usually skim the patch looking for huge mistakes, ones that will make the patch worthless.  If I find one, I r- the patch and add a comment saying why.  Otherwise I try to check whether I can review the patch quickly or not.  If this looks like a quick review, I just do it before anything else.  Otherwise I leave a tab with that patch open and put it off until later during the day.

Later during the day when I get some free time, I start reviewing those remaining patches as I multitask reading email or news.  If I see that I can’t review a given patch shortly enough, I contact the author and let them know, and give them an estimate of when I can get back to them, and ask them to let me know if they’re in a hurry or if that’s not acceptable.  I usually try to either find the author in person or over IRC, and if I can’t reach them, I either send them an email and describe the situation.  Sometimes unfortunately (as is now the case with bug 617532) I can’t do the review for a considerable amount of time (usually because the review involves something like reading an entire spec and keep it in mind), in which case I let the author know about the situation, and work out how we should proceed.  Under no circumstances I let a patch to not receive some sort of feedback after I see it.

Throughout the day as I read my bugmail, I look for new review requests, and handle them similarly to the above.  Sometimes if I don’t have enough time to keep a close eye over my bugmail, I won’t see the review requests for the day, in which case they’re brought to my attention by Bugzilla tomorrow morning.

I started to do this in June 2010.  There have been many consecutive days as I was fighting some kind of a fire, I was not able to look at my bugmail at all.  When the pressure of other work drops off, the whine emails help me do my pending review requests first thing in the next morning.  The review requests that I can’t deal with indefinitely don’t come up very much (I can think of only 2 cases in the past 6 months) so this system has worked great for me.  If you don’t consider outliers such as those two cases, I would expect that my maximum response time on review requests is below 12 hours, and on good days, my average response time should be quite lower than that (unless someone asks me for review right before I go to sleep!).

Doing reviews faster helps really well to get more review requests, and some may see that as a downside, but I think that code review is one of the most important ways you can contribute to Mozilla, so I’m definitely proud that that has been the case for me.

Posted in Blog Tagged with: ,