Mailing-List: contact dev-help@forrest.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@forrest.apache.org
Received-SPF: pass (herse.apache.org: domain of ross.gardler@googlemail.com
 designates 209.85.146.181 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=googlemail.com; s=beta;
        h=received:message-id:date:from:sender:to:subject:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth;
        b=BmrNIA0LE3BPFYrtSRnveEkJKkC6B8nGV8+Q9frGOIZlQfi3GE3GUB719gZ7Fgvrw3J9Oq6D1RK0cP29geRTyWRVthEp6jT2UT8GVjsvbWKvuZtqZ2XOGp9oPwrysJ/XNf0byOZyGO5eEsg+bIU7/5EshlC2PXU1kFJgtie2ZNs=
Message-ID: <61c9bc470706070321q7d0d39b3m29a3e37deac370f6@mail.gmail.com>
Date: Thu, 7 Jun 2007 11:21:26 +0100
From: "Ross Gardler" <rgardler@apache.org>
Sender: ross.gardler@googlemail.com
To: dev@forrest.apache.org
Subject: Re: Commit then review revisited.
In-Reply-To: <20070607000507.GE540@igg.indexgeo.com.au>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <008001c7a890$c2773de0$650fa8c0@developer>
	 <20070607000507.GE540@igg.indexgeo.com.au>

On 07/06/07, David Crossley <crossley@apache.org> wrote:
> Gav.... wrote:

...

> > Anyway, just wanted to nip this in the bud now, should I from now on revert
> > my thinking and review the patches before committing, even though this will
> > mean the patch just waits there a little longer ?

...

> No, stick with "commit-then-review".

I agree.

[...]

> Doing "commit-then-review" does not mean "don't look at it
> beforehand".

Yes, in this case a quick review of the patch would have revealed that
the patch itself was faulty and so we could have asked for a new
patch. This would have been much faster than the time it took me to
figure out what had gone wrong and to edit each file individually by
hand.

How much reviewing would you do of your own work before committing? I
guess you would at least check that it worked and then commit. In this
case just trying to run the code locally would have indicated a
problem.

> It is up to the patch applier to consider how much pre-commit
> review that they feel like doing.

Agreed, but at the very least, read it over to make sure it is
correctly put together. Especially when it is an early patch since
creating patches correctly is a fine art, I don't know anyone who has
not made a few errors at first.

Finally, as ever, no-one should feel that this is a problem in an open
source community. We all make mistakes and all pick up the pieces for
one another. I only highlight the issue in order to fine tune the
process.

So, thanks for taking the time to figure out how to apply the patch in
the first place.

Ross