From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Fri, 27 May 2011 07:22:07 GMT
Subject: Re: [PATCH v4 00/10] Sequencer Foundations
Message-ID: <BANLkTik1oyCCjcAqvqZNHPecNKUJ9k0bCA@mail.gmail.com>
In-Reply-To: <7vpqn5w9zi.fsf@alter.siamese.dyndns.org>

Hi Junio,

Junio C Hamano writes:
> 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.

> 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