[development] What about reviewing patches?
catch56 at googlemail.com
Wed Aug 13 20:45:07 UTC 2008
On Wed, Aug 13, 2008 at 9:08 PM, Katherine Senzee wrote:
> On Wed, Aug 13, 2008 at 12:52 PM, Nathaniel Catchpole
>> Some patch reviews take less than 30 seconds, especially when the
>> issue queue is long and has lots of stale patches in it.
> I'm not sure I follow. I review patches when I get the time, but I
> figure on at least 20 minutes -- read the issue, check out HEAD, apply
> the patch, create a new database, install the test site,
> download/install devel, generate stuff, see if the patch works as
> advertised, post my review. If I have a recent enough HEAD install I
> figure I can skip the database bit and just use my existing database,
> but it's still a decent chunk of time. What am I doing wrong?
20 minutes is about right to do a proper review, but a lot of patches
don't even apply any more, including RTBC ones.
So if you already have HEAD installed (I pretty much only clean my
database before or after reviewing a patch with an upgrade path or
schema change), you can do this:
// clean your HEAD of old changes.
cvs -q diff | patch -p0 -R
patch -p0 < somepatch.patch
1 out of 1 hunks failed etc.
Mark to code needs work with 'needs a re-roll'.
Maybe 45 seconds then, but not too much more than that.
Also simpletest is a much, much easier and more comprehensive way to
avoid regressions that installing devel, generating content and
clicking around. So that stage is reduced to: admin/build/modules
admin/build/testing run all tests
Although you can go make a cup of tea while the tests run, there's
little effort involved running them. You should obviously test the
actual functionality of the patch on top of this, but that's a bit
less RSI inducing.
More information about the development