I would love to see a series of blog posts from Drupal developers on patch review strategies they employ, so we can share some tips and tricks and ramp up our collective review IQs. I'll start it off with mine. I call it "The 6-Pass Patch Review" (wittier names welcome! ;))

To begin, crack open your text editor. You'll use it to jot down notes and questions as you go through each of these passes. Other than that, no fancy tools are required to do most of this other than a keen set of eyes, an inquisitive mind, and the ability to empathize with people new to Drupal and put yourself in their shoes. (Of course, if you are relatively new to Drupal, then patch review can actually come easier for you than other people. :) Start today!)

As a standards zealot, one of the things I'd personally love to see happen in Drupal 7 is the eradication of table-based layouts. I'm giving stink-eye directly at you, Pushbutton, Chameleon, and Bluemarine. These themes are a strange abberation from the Drupal project's otherwise very meticulous attention to detail regarding web standards, and I'm quite positive that they directly contribute to the Drupal project's ongoing struggles to attract and retain visual designers. Garland is a huge step up, but it was never designed to be an easy to modify base theme, and unless designers happen to stumble across something like Zen, I imagine they come away thinking that Drupal's markup is ugly and antiquated and go to something a bit nicer out of the box like WordPress.

I want Drupal 7 to be an incredible release that ramps up the user experience by 1,000-fold, and making Drupal more accessible to designers and themers (along with an accompanying selection of great looking designs out of the box) is a huge part of that. Fortunately, I'm not alone!

In coordination with several prominent members of Drupal's design/theming community, including Joon Park, John Wilkins, Brad Bowman, Stephanie Pakrul, and Earl Miles, we've come up with what we think is a workable plan for accomplishing these goals.

The overall gist is:

1. Replace Drupal's default markup with a flexible, standards-based framework that's easy to extend.
2. Hold a theming contest which uses the base markup, and select the best N designs to go into core as sub-themes.
3. Remove the old, crufty themes and replace them with the new, gorgeous ones.
4. Profit!

So if you either a) are a web standards zealot who wants to lend your expertise to the discussion on selecting the default markup, or b) have some time / energy / etc. to help organize a theming contest, please coordinate over at http://groups.drupal.org/node/16200!

Today was the first day of my vacation (linking to Wikipedia in case you, like me, were not sure what this word meant) so naturally a bunch of us spent the entire day in #drupal slashing through bugs and features in preparation of DRUPAL-7-0-UNSTABLE-2 which should be tagged tomorrow, with lots of nice goodies. :)

The result? The queue of patches ready to be committed queue as I sign off here is down to zero issues*.

Thanks to everyone in #drupal who participated, with a special shout-out to Dave Reid, who spent the entire day (and evening, and early next morning) testing and rolling (and re-rolling, and re-re-rolling) patches, Peter Wolanin for getting all the various D6 security patches ready, Matt Farina for sticking it out while we debugged an 11th hour issue, and the folks working away at BADCamp for their Testing Party patches! Yeah!! :)

Just a heads-up; I'm going to be afk from Tuesday, October 14 to Monday, October 20 (/me gets the jitters already). Hopefully there'll be a nice big list of patches to crank through when I get back. :)

Let's keep rockin' that Patch Spotlight and get all kinds of more goodies ready for UNSTABLE-3 in another couple of weeks!

As of this writing, there are over 5,600 active issues in the Drupal core queue. About half of those are listed in the 2,650 active issues for Drupal 7. And the "patch queue for Drupal 7 -- those issues that have at least *some* code to fix them in various stages of completion -- clocks in at 1,040 issues.

These numbers are, quite frankly, staggering. Discouraging, even, some might say. With so many issues out there vying for attention, how are we to find those ones that are really important, such as killer new features, powerful API enhancements, and critical bug fixes? While Drupal 7's development cycle doesn't have an expiration date on it yet, we all know that day will come, and probably sooner than we'd ideally like. What can we do to make sure that important patches are raised to the surface and receive adequate developer and committer attention?

To succeed, we'll need to employ a two-pronged approach: taking out the trash, and digging up the treasure.

Here's another tip for those who are looking for more reviews of their patches and faster commits: describe what the heck your patch does! :)

A simple enough premise, and It sounds obvious, right? But you'd be surprised how few patches are accompanied by a good, solid list of changes.

For a really bad example of this, allow me to pick on our #1 core patch contributor for a moment. Witness http://drupal.org/node/259412. The issue title is "Total module system revamp." The description? Two words: "Drupal diet." Accompanied by a 30K patch that removes and adds all manner of things. Please DO NOT do this. :P Doing this means that anyone who wants to review that patch (or commit it) has to sit down for at least an uninterrupted hour and sift through the diff to figure out what's even going on, before they can even begin to do the actual review to answer things like, "Is this the best way to solve this? Are there additional places we could make improvements? Does the code introduce logic errors?" and so on. Since this is a daunting effort, it means that often times reviewers simply don't bother looking any further at such patches, and so they end up rotting in the queue for months on end.

Most patches aren't this extreme, but they could almost all universally stand to be improved. Let's try to make sure every patch in the queue has the following:

Patch authors are always looking for a strategies to help shorten the length of time it takes to go from creating a patch to seeing it reviewed to getting it committed. Here's a quick tip for any patches that involve a UI change: Submit a screen shot with your patch!

Screen shots present a really easy way for anyone, regardless if they are a developer who has a Drupal 7 development environment setup or not, to quickly scan the gist of what the patch does and to provide feedback on whether things need to be changed. And in fact, those people who don't have Drupal 7 development environments are actually the people whose UI feedback we need the most! :)

Naturally, you can also go above and beyond, such as creating an entire video demo. ;) But in most cases, a simple before/after screen shot is sufficient.

So, a little challenge for Drupal core developers out there: let's strive to make sure every UI-changing patch in the queue accompanied by a screen shot. It's an easy way to get more eyes on your stuff, get more types of eyes on your stuff, and sort out the challenges well ahead of commit time.

We've all been there. You sit down to write a "simple" patch. Maybe it upgrades a module to Drupal 6. Maybe it adds a small feature, or fixes a bug that's been annoying you for awhile.

You grab your caffeinated beverage of choice, pop open your text editor/IDE, and throw on your coding tunes. You search through the file for the lines you need to change and... sayyy... You know, the indentation is off a bit here. I'll just go ahead and fix that. Oops. And this function's PHPDoc is missing. No problem, a couple quick lines here. OH! That's clearly a bug. Better fix that while I'm at it. Hey, and that reminds me! I always kind of wished that this particular feature worked a different way. Might as well pop that in while I'm at it...

Suddenly your "simple" patch has grown into 30-40K of changes. It fixes the original thing you set out to fix, and it also makes several improvements to the code that you found as you were going through. Great! So what's the problem?

The problem is context switching.