Re: [PATCH] config: Use parseopt.

On Sat, Feb 14, 2009 at 11:10 PM, Junio C Hamano [off-list ref] wrote:

Felipe Contreras [off-list ref] writes:

quoted

quoted

Unfortunately, not many patch authors write such a summary.  Sometimes we
see summaries on things that were discussed but nobody has followed
through posted by third parties (including myself), but we do not seem to
have enough helpers to do that either.  This does not take much technical
skills but is a good "trust point" earner.

For me it's easier, and more fun to write a separate patch that fixes
the issues than writing a summary,...

That certainly is something we should take into consideration.

I however think an unwritten assumption around here so far has been that
the patch author who gets review comments is expected to keep track of the
issues raised, both about the patch itself and about the similar breakages
in the existing code pointed out during the review process, if only
because the patch author is the focal point of the discussion.

We probably need to break that.

Because it is very likely that the reviewer does not even realize that
such similar breakages in the existing code when a review is made, we
cannot ask reviewers to always start a separate discussion.  Some reviews
do say "Admittedly, we already have the same pattern in here and there,
but this in your patch is wrong," but the way how we collectively realize
an existing breakage is often by hearing the patch author respond with
"but there already are this and that breakages in the existing code."

We do not want such knowledge of existing breakages go to waste in either
case.  Perhaps it would be a good start to make it the responsibility of
the first person who mentions an existing breakage (either the reviewer's
"Admittedly", or the patch author's "but there already are") to begin a
separate thread, so that mail archive would remember it.  It shouldn't
take more than 3 minutes.

That is true, however I propose something a bit different. At least in
this review there has been a number of issues brought up, it would be
overkill to create a separate thread for each one of these issues as
they where found, and if the patch submitter is new, he probably
wouldn't know about this rule.

So, I propose that at the end of the patch review process the ack
person (or somebody else) asks the patch submitter (possibly cc'ing
the reviewers) to start a new thread mentioning the pending issues
brought up in the review.

-- 
Felipe Contreras