Re: [PATCH v4 00/10] Sequencer Foundations

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