Patch review sprints

What is a patch review sprint?

A patch review sprint is getting people together for a set amount of time – anywhere from a couple of hours to a couple of days – and just reviewing patches (see what is a patch?).

Patch review sprints offer a great learning experience, as they help expose people to many parts of Drupal they may not otherwise have had access to. It's also a lot of fun, as it gets folks talking and working directly with Drupal core contributors and having a rompin' stompin' good time making Drupal better!

This document describes how patch reviews work, the list of prerequisites you need in place in order to help, and a helpful cheat-sheet of commands for use while patch reviewing. Please note that "coding skills" is not on the list of prerequisites. While people with coding skills can perform certain types of reviews better than non-coders, non-coders can also perform certain types of reviews better than coders. In short: everyone is welcome!

How It Works

The goal is to try and knock the core patches to review queue down to zero (or as close to it as possible). Choose an issue from the list that seems interesting, and apply the patch to test it out. If you're looking for a patch to review in a specific area of interest, you can also glance through the Drupal core community initiatives page. And the novice issue queue contains issues that only change minor things and should be fairly easy to review if you're just getting your feet wet.

As a patch reviewer, your primary objective is to find a reason to mark a patch "needs work." Some reasons for doing so include:

• The patch has grammar or spelling mistakes
• The patch does not work when viewed in browsers such as Safari, IE, or Opera.
• The patch causes flaming errors when applied to your copy of Drupal.
• When trying something crafty to out-smart the original patch author, you come across unintended behaviour.
• The resulting user interface is obtuse and difficult to use.
• The code doesn't conform to coding standards.
• The changes are insufficiently documented and/or difficult to understand.
• The patch does not contain tests to prove a new feature works, and/or a bug stays fixed.

If, and only if, you can find nothing wrong with the patch after trying really hard to do so, then you begrudgingly mark it "reviewed & tested by the community."

Regardless of the outcome of the patch review, document carefully in your reply all of the things you tried and what results you achieved, things you liked about the patch, things you did not like, and so on. A good, solid patch review is typically 2-3 paragraphs or several bullet points. "+1" is NOT a patch review.

Remember to make sure that your comment reflects any conversation that might've happened in IRC, in a conversation over lunch, etc. If it's not in the issue queue, it never happened. :)

There are more tips on performing patch reviews at Reviewing patches part of the larger Help in testing code part of the handbook.

Prerequisites

In order to participate in a patch review sprint, you'll need the following:

• An inquisitive mind with an attention to detail. Extra bonus if you are one of those people with an uncanny ability to break things.
• A development environment running at least PHP 5.2. An easy way to get this is to install a local Apache/MySQL/PHP package such as XAMPP. There are helpful videos for Windows, Mac, and Ubuntu Linux
• The 'patch' command-line utility. If you type in
  patch --help
  you should see a bunch of text. If you do not, and instead see something like "Command not found," you do not have patch. You are also probably on Windows. Watch this helpful video.
• (optional, but helpful) The 'cvs' command-line utility, or a graphical equivalent. If you type in
  cvs --help
  from the command-line, you should see a bunch of text. If you see something like "Command not found," and you aren't using something like TortoiseCVS, you do not have cvs. Here's how to set it up on Mac and Windows.
• (optional, but helpful) If you're not familiar with patching yet, watch a video on how to apply patches to get a quick overview. Otherwise, you'll pick it up soon enough. :) There are also specific instructions on applying patches in Windows and Mac.

Patch review cheatsheet

1. Download and install a copy of Drupal 7. You can either download the latest 7.x-dev release or (optional, but recommended) get a cvs checkout of Drupal HEAD:
   
   cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal checkout -d 7x drupal
2. Find an issue that needs review from the "Drupal" project issue queue.
3. Give a quick read through the patch (even if you're a non-coder) and look for anything obviously wrong that sticks out to you.
4. From the issue, download the patch file to the "root" of your local Drupal installation (the folder with INSTALL.txt and cron.php in it). You can either do this with your browser or from the command line with a tool like wget:
   
   cd /path/to/htdocs/drupal7
wget http://drupal.org/files/issues/example.patch
5. Apply the patch, using the patch utility:
   
   patch -p0 < example.patch
6. Click around your Drupal interface and try and break the patch. Try and think of things that the original author may not have intended; for example, testing OpenID on a patch that affects the user registration form.
7. If you run into a bug, see if the same bug exists when you revert the patch:
   
   patch -p0 -R < example.patch
8. Post your results to the issue, as described above in "How it works."
9. Now, get back to a "clean" copy of Drupal 7 and go back to step 2! If using the 7.x-dev release, you can delete the Drupal 7 folder and re-expand the archive to put it back. If you're using CVS you can do:
   
   cvs up -dPC