Re: [PATCH v4 00/10] Sequencer Foundations
- From
Ramkumar Ramachandra <artagnon@gmail.com>
- Date
- May 27, 2011, 07:22 UTC
- Message-ID
- <BANLkTik1oyCCjcAqvqZNHPecNKUJ9k0bCA@mail.gmail.com>
- In-Reply-To
- <7vpqn5w9zi.fsf@alter.siamese.dyndns.org>
Hi Junio,
Junio C Hamano writes:
Show 8 quoted lines
> 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".
You're right; this iteration was ill-thought-out. I'll try to put myself in my reviewer's shoes before submitting code next time, so that I don't waste effort re-rolling unnecessarily.
Show 9 quoted lines
> 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?
Oops, I'll rebase. I'll trace all the code paths carefully and include a summary of my learnings in the next iteration.
-- Ram