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

Re: [PATCHv3 0/5]rev-list and friends: --min-parents, --max-parents

From
Jeff King <peff@peff.net>
Date
Mar 23, 2011, 14:48 UTC
Message-ID
<20110323144835.GA30337@sigill.intra.peff.net>
In-Reply-To
<cover.1300872923.git.git@drmicha.warpmail.net>
On Wed, Mar 23, 2011 at 10:38:47AM +0100, Michael J Gruber wrote:
> Compared to what is currently in pu (which is v2+eps), v3 has:
This one looks good to me (I like the --no-{min,max}-parents).
Show 17 quoted lines
> 1/3 -> 1/5 unchanged
> 
> 2/3 -> 2/5 unchanged
> 
> 3/5 is !squash for 2/5 and introduces --no-min-parents and --no-max-parents
> as natural ways to reset the limits
> 
> 3/3 -> 4/5 with a fix to the notation in documentation (spell out =<number>)
> and an additional dodecapus test
> 
> 5/5 is !fixup for 4/5 and adds test, doc and completion for --no-min-parents
> and --no-max-parents
> 
> Junio, please let me/us know whether sending an amended series in this way
> (which I've seen before) is actually convenient for you or not. !squash
> commits require a message edit, for example. OTOH, I don't know any (other?)
> good inter diff solution.

As a reviewer, I find it is usually most convenient to just have the submitter do the squash, and then write below the "---" (or in a cover letter like this) a brief explanation of the differences.

Yeah, I end up reading the whole patch again, including bits I already read, but that is probably a good thing. A good patch is hopefully not too long, and it is easier (to me, anyway) to review it as a whole and not as an interdiff.

Technically speaking, what you posted gives more information, and I could always apply and squash myself to get the final form. But I'm lazy and I tend to review straight from the mail reader or from $EDITOR while replying. So to me it's just an extra step.

I have occasionally seen people with long patches reply to the patch themselves and say "btw, it is hard to see what is changed here in v2, so here is a much more readable interdiff from v1". And that can be useful at times, but it's nice to have the patch author using their judgement about when it will be useful or not.

Just my two cents.
> Thanks for everyone's input which went into this (Junio, Jeff, Jonathan).
Thank you for taking the lead on writing it. :)
-Peff
Previous: Michael J GruberNext: Junio C Hamano
Message 17 of 67 in “revision.c: --merges, --no-merges and --merges-only”
  1. 0/2 revision.c: --merges, --no-merges and --merges-onlyMichael J Gruber, Mar 17, 2011
  2. 1/2 revision.c: rename --merges to --merges-onlyMichael J Gruber, Mar 17, 2011
  3. 2/2 revision.c: introduce --merges to undo --no-mergesMichael J Gruber, Mar 17, 2011
  4. Junio C HamanoMar 17, 2011
  5. Jeff KingMar 17, 2011
  6. Michael J GruberMar 18, 2011
  7. Junio C HamanoMar 18, 2011
  8. Michael J GruberMar 18, 2011
  9. Jeff KingMar 18, 2011
  10. 0/3 rev-list and friends: --min-parents, --max-parentsMichael J Gruber, Mar 18, 2011
  11. Michael J GruberMar 18, 2011
  12. Jeff KingMar 18, 2011
  13. Michael J GruberMar 21, 2011
  14. 0/3 rev-list and friends: --min-parents, --max-parentsMichael J Gruber, Mar 21, 2011
  15. Jeff KingMar 21, 2011
  16. 0/5 rev-list and friends: --min-parents, --max-parentsMichael J Gruber, Mar 23, 2011
  17. Jeff KingMar 23, 2011
  18. Junio C HamanoMar 23, 2011
  19. Junio C HamanoMar 23, 2011
  20. Jonathan NiederMar 24, 2011
  21. Michael J GruberMar 24, 2011
  22. Jonathan NiederMar 24, 2011
  23. 0/2 test_when_finished and returning earlyJonathan Nieder, Aug 8, 2011
  24. 1/2 test: simplify return value of test_run_Jonathan Nieder, Aug 8, 2011
  25. 2/2 test: cope better with use of return for errorsJonathan Nieder, Aug 8, 2011
  26. Johannes SixtAug 9, 2011
  27. Jeff KingAug 9, 2011
  28. t3900: do not reference numbered arguments from the test scriptJohannes Sixt, Aug 11, 2011
  29. Jonathan NiederAug 11, 2011
  30. Junio C HamanoAug 11, 2011
  31. Jeff KingAug 8, 2011
  32. 1/5 t6009: use test_commit() from test-lib.shMichael J Gruber, Mar 23, 2011
  33. 2/5 revision.c: introduce --min-parents and --max-parentsMichael J Gruber, Mar 23, 2011
  34. 3/5 squash! revision.c: introduce --min-parents and --max-parentsMichael J Gruber, Mar 23, 2011
  35. 4/5 rev-list --min-parents,--max-parents: doc, test and completionMichael J Gruber, Mar 23, 2011
  36. 5/5 fixup! rev-list --min-parents,--max-parents: doc, test and completionMichael J Gruber, Mar 23, 2011
  37. 1/3 t6009: use test_commit() from test-lib.shMichael J Gruber, Mar 21, 2011
  38. 2/3 revision.c: introduce --min-parents and --max-parentsMichael J Gruber, Mar 21, 2011
  39. Michael J GruberMar 21, 2011
  40. Junio C HamanoMar 21, 2011
  41. Jonathan NiederMar 21, 2011
  42. 3/3 rev-list --min-parents,--max-parents: doc and test and completionMichael J Gruber, Mar 21, 2011
  43. Jonathan NiederMar 21, 2011
  44. Michael J GruberMar 22, 2011
  45. Jonathan NiederMar 23, 2011
  46. 1/3 revision.c: introduce --min-parents and --max-parentsMichael J Gruber, Mar 18, 2011
  47. Jeff KingMar 18, 2011
  48. Michael J GruberMar 21, 2011
  49. Jonathan NiederMar 18, 2011
  50. Junio C HamanoMar 18, 2011
  51. Michael J GruberMar 21, 2011
  52. 2/3 t6009: use test_commit() from test-lib.shMichael J Gruber, Mar 18, 2011
  53. 3/3 rev-list --min-parents,--max-parents: doc and test and completionMichael J Gruber, Mar 18, 2011
  54. Jeff KingMar 18, 2011
  55. Michael J GruberMar 21, 2011
  56. Jeff KingMar 21, 2011
  57. Michael J GruberMar 21, 2011
  58. Junio C HamanoMar 21, 2011
  59. Michael J GruberMar 21, 2011
  60. Junio C HamanoMar 21, 2011
  61. Jonathan NiederMar 18, 2011
  62. Michael J GruberMar 21, 2011
  63. Jonathan NiederMar 21, 2011
  64. Michael J GruberMar 22, 2011
  65. Jeff KingMar 18, 2011
  66. Michael J GruberMar 18, 2011
  67. Jeff KingMar 18, 2011

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.