git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] config: Use parseopt.

From
Felipe Contreras <felipe.contreras@gmail.com>
Date
Feb 14, 2009, 21:24 UTC
Message-ID
<94a0d4530902141324w1cea462ex99a698e5a702d85a@mail.gmail.com>
In-Reply-To
<7vtz6wrahg.fsf@gitster.siamese.dyndns.org>
On Sat, Feb 14, 2009 at 11:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 35 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> 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
Previous: Junio C HamanoNext: Johannes Schindelin
Message 40 of 45 in “config: Use parseopt.”
  1. config: Use parseopt.Felipe Contreras, Feb 14, 2009
  2. Junio C HamanoFeb 14, 2009
  3. Junio C HamanoFeb 14, 2009
  4. Felipe ContrerasFeb 14, 2009
  5. Felipe ContrerasFeb 14, 2009
  6. Johannes SchindelinFeb 14, 2009
  7. config: Use parseopt.Felipe Contreras, Feb 14, 2009
  8. Jeff KingFeb 14, 2009
  9. Felipe ContrerasFeb 14, 2009
  10. Johannes SchindelinFeb 14, 2009
  11. Junio C HamanoFeb 14, 2009
  12. human readable diffs, was Re: [PATCH v2] config: Use parseopt.Johannes Schindelin, Feb 14, 2009
  13. Felipe ContrerasFeb 14, 2009
  14. Johannes SchindelinFeb 14, 2009
  15. Jakub NarebskiFeb 14, 2009
  16. Johannes SchindelinFeb 14, 2009
  17. Felipe ContrerasFeb 15, 2009
  18. Johannes SchindelinFeb 15, 2009
  19. Felipe ContrerasFeb 15, 2009
  20. Johannes SchindelinFeb 15, 2009
  21. Felipe ContrerasFeb 15, 2009
  22. Felipe ContrerasFeb 15, 2009
  23. Junio C HamanoFeb 15, 2009
  24. Johannes SchindelinFeb 15, 2009
  25. Junio C HamanoFeb 15, 2009
  26. Johannes SchindelinFeb 15, 2009
  27. Felipe ContrerasFeb 17, 2009
  28. Junio C HamanoFeb 15, 2009
  29. Felipe ContrerasFeb 14, 2009
  30. Johannes SchindelinFeb 14, 2009
  31. Felipe ContrerasFeb 14, 2009
  32. Johannes SchindelinFeb 14, 2009
  33. Johannes SchindelinFeb 14, 2009
  34. Felipe ContrerasFeb 14, 2009
  35. Junio C HamanoFeb 14, 2009
  36. Felipe ContrerasFeb 14, 2009
  37. Junio C HamanoFeb 14, 2009
  38. Felipe ContrerasFeb 14, 2009
  39. Junio C HamanoFeb 14, 2009
  40. Felipe ContrerasFeb 14, 2009
  41. Johannes SchindelinFeb 14, 2009
  42. Junio C HamanoFeb 15, 2009
  43. Jakub NarebskiFeb 14, 2009
  44. Felipe ContrerasFeb 14, 2009
  45. Jeff KingFeb 14, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.