On Sat, Feb 14, 2009 at 11:10 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Felipe Contreras <felipe.contreras@xxxxxxxxx> writes: > >>> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
