forrest-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ross Gardler" <>
Subject Re: Commit then review revisited.
Date Thu, 07 Jun 2007 10:21:26 GMT
On 07/06/07, David Crossley <> 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

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

> 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

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


View raw message