kfogel@newton.ch.collab.net wrote on 07/25/2005 11:17:06 AM:
> I'll post a separate mail to the dev@ list, recommending some specific
> ways we can make their patch reviews less intimidating, and to focus
> more on substantive stuff. I believe yours and Mark's overall point
> is a good one. We have been a bit too high-priestish over here in the
> temple of Subversion, and that's not good for contributors or for the
> project.
This seems like as good a post as any to respond to. First, I would like
to thank those that responded to my post for taking it as constructive
criticism and not an attempt to flame anyone. I just wanted to clarify my
own comments. I think that Valik had a few good points, but mostly went a
lot further in his criticisms then I intended.
Personally, I like all of the style comments and the fact that the code is
reviewed so thoroughly. It is one of the things that makes the code
maintainable. Who would want to read a book where each paragraph was
written by a different author with no common editor? My point was that
there are many different kinds of posts that come across this list, and
they tend to all be treated the same.
I think there is a problem that no one has pointed out yet here, and that
is that once you start offering style review, or even a review that gets
into some of the substance there is almost an implicit acceptance that the
patch itself is worthy of inclusion. If it later turns out that there are
some legitimate exceptions, then it leads to more hard feelings than were
necessary. "Why did you make me cleanup and resubmit a patch 3 times only
to then tell me there is no way you would commit it?"
There are no easy answers. I am sure we are all trying to do the best we
can and clearly none of the committers in this project are intentionally
trying to upset potential contributors. I think if you are inclined to
review a patch the first thing you should ask yourself is whether you have
all of the "meta-information" you need to do so. Does the log message or
the email include enough of this information so that you are clear about
and agree with the objectives? If not, then the first review comments
should probably just be to collect that information and engage that
discussion. Everything else should be able to flow from there.
Thanks
Mark
_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Jul 25 19:17:14 2005