On 22-May-08, at 12:13 AM, Andrew Savory wrote:
Sure, Commit-Then-Review vs. Review-Then-Commit ... but I don't
actually think RTC is going to ensure significantly more widespread
review since the time burden on other developers to find the issue in
JIRA, download the patch, apply the patch, test, respond, then revert
the change. Do people really have the time to do that? It's
significantly more effort than that to svn update, look at code, and
feed back. I prefer detailed discussion on the mailing list (which
supports decent threading, quoting etc, unlike JIRA) followed by
commit of a trial implementation which can then be refactored.
Otherwise there might be a tendency to analysis paralysis. But I'm the
new boy here, so I'll STFU and try to help out on the release instead
of forcing y'all to rehash old discussions on how to run an open
source project ;-) Maybe by the time 1.3 is out the door we'll all be
using distributed SCM systems and the discussion will be moot anyway!
I think we agree in principle--a patch does not have to be spotless to
be committed. I also agree that that mailinglist is a preferable
place to hash out design details. But it is necessary that the basic
approach is one we feel will stick with before getting committed. I
don't think this imposes much of a burden on people aiming to review a
patch.
It is true that using patches takes an extra minute or two to set up,
but the time to evaluate a contribution is _by far_ mostly contained
in understanding the contribution, its implications, and examining the
code. Plus, the patch is much easier to back out of a given
repository and makes it easier to see exactly what changes were made.
Since contributors can't commit to the repository anyway, I don't see
much disadvantage in working with patches.
(btw, if you want a one-line equivalent to "svn up", try something like:
$ wget http://issues.apache.org/jira/secure/attachment/12381498/SOLR-563.patch
-O - | patch -p0
Reverting is also one line:
$ svn revert -R .
Although this leaves added files, which can be removed with
$ svn st | grep '?' | awk '{print $2}' | xargs rm
Another useful trick is to have multiple checkouts of trunk and
"bounce" an active changeset from one to another with
$ svn diff | (cd ../otherbranch; patch -p0)
)
-Mike
-Mike
