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

Re: [PATCH v4 00/10] Sequencer Foundations

From
Junio C Hamano <gitster@pobox.com>
Date
May 26, 2011, 19:13 UTC
Message-ID
<7vpqn5w9zi.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1306425233-504-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> This is a quick re-roll after Junio's review of the previous
> iteration; the previous iteration was posted less than 24 hours ago.
> I'm in a hurry,...

Open source is not just about pushing new code through other people's throats, but also about the skill to make other people around you to work with you. They may be busy spending time on helping other topics moving forward. Or they may simply be bone-headed ;-).

If you are in a hurry, try to spend _more_ time making sure it is easier to review for other people. What were updated, what were kept as they were and for what reason? The purpose of adding the cover letter to the series is to do just that.

Without spending that effort and saying "I am in a hurry" give a wrong impression to others: "I cannot tell if this round is better cooked than the previous round without spending nontrivial time reading on it, but chances are it hasn't been much improved during the 24 hours. It may be better use of my time to skip it and work on other topics first. After finishing everything else I can come back---by that time Ram may have sent out another round and reviewing this round right now may become wasted time".

Also the series seems to be based on a rather old codebase where we didn't allow reverting the root commit. I tried to apply a first few to understand where these "positive" error status could be coming from, and was stopped by patch conflicts. In [2/10] I think you are propagating errors from run_command() coming back through try_merge_command(), and the change is good for that particular codepath, but I haven't followed all the other codepaths from do_cherry_pick() to convince myself that the exit status from the merge strategy backend are _the only_ positive returns you can possibly get. Have you?

Previous: Ramkumar RamachandraNext: Ramkumar Ramachandra
Message 36 of 37 in “Sequencer Foundations”
  1. 00/10 Sequencer FoundationsRamkumar Ramachandra, May 25, 2011
  2. 01/10 advice: Introduce error_resolve_conflictRamkumar Ramachandra, May 25, 2011
  3. 02/10 revert: Propogate errors upwards from do_pick_commitRamkumar Ramachandra, May 25, 2011
  4. Junio C HamanoMay 25, 2011
  5. Ramkumar RamachandraMay 26, 2011
  6. 03/10 revert: Eliminate global "commit" variableRamkumar Ramachandra, May 25, 2011
  7. Junio C HamanoMay 25, 2011
  8. 04/10 revert: Rename no_replay to record_originRamkumar Ramachandra, May 25, 2011
  9. 05/10 revert: Introduce struct to keep command-line optionsRamkumar Ramachandra, May 25, 2011
  10. 06/10 revert: Separate cmdline parsing from functional codeRamkumar Ramachandra, May 25, 2011
  11. 07/10 revert: Catch incompatible command-line options earlyRamkumar Ramachandra, May 25, 2011
  12. 08/10 revert: Introduce HEAD, TODO files to persist state, planRamkumar Ramachandra, May 25, 2011
  13. 09/10 revert: Implement parsing --continue, --abort and --skipRamkumar Ramachandra, May 25, 2011
  14. 10/10 revert: Implement --abort processingRamkumar Ramachandra, May 25, 2011
  15. Junio C HamanoMay 25, 2011
  16. 00/10 Sequencer FoundationsRamkumar Ramachandra, May 26, 2011
  17. 01/10 advice: Introduce error_resolve_conflictRamkumar Ramachandra, May 26, 2011
  18. 02/10 revert: Propogate errors upwards from do_pick_commitRamkumar Ramachandra, May 26, 2011
  19. 03/10 revert: Eliminate global "commit" variableRamkumar Ramachandra, May 26, 2011
  20. 04/10 revert: Rename no_replay to record_originRamkumar Ramachandra, May 26, 2011
  21. 05/10 revert: Introduce struct to keep command-line optionsRamkumar Ramachandra, May 26, 2011
  22. 06/10 revert: Separate cmdline parsing from functional codeRamkumar Ramachandra, May 26, 2011
  23. 07/10 revert: Catch incompatible command-line options earlyRamkumar Ramachandra, May 26, 2011
  24. 08/10 revert: Introduce HEAD, TODO files to persist state, planRamkumar Ramachandra, May 26, 2011
  25. Jonathan NiederMay 26, 2011
  26. Ramkumar RamachandraMay 26, 2011
  27. Ramkumar RamachandraMay 26, 2011
  28. Junio C HamanoMay 26, 2011
  29. Ramkumar RamachandraJun 1, 2011
  30. Jonathan NiederJun 1, 2011
  31. Ramkumar RamachandraJun 2, 2011
  32. Jeff KingJun 2, 2011
  33. Ramkumar RamachandraJun 8, 2011
  34. 09/10 revert: Implement parsing --continue, --abort and --skipRamkumar Ramachandra, May 26, 2011
  35. 10/10 revert: Implement --abort processingRamkumar Ramachandra, May 26, 2011
  36. Junio C HamanoMay 26, 2011
  37. Ramkumar RamachandraMay 27, 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.