From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Wed, 11 Jan 2012 09:19:56 GMT
Subject: Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
Message-ID: <CALkWK0=o+KkV08G9JuTaG8Vtb-AzHQVMQPzMy8td_iBVnGX4Dg@mail.gmail.com>
In-Reply-To: <20120111054954.GB13507@burratino>

Jonathan Nieder wrote:
> Well, let's start here.  The two insns "pick" and "revert" and the
> ability to mix them doesn't have much to do with the picture, does it?
>
> I think the actual problem being solved is that insn types, as described
> by the replay_action enum, are being abused to refer to top-level git
> commands "revert" and "cherry-pick".  The sequencer isn't supposed to
> care which top-level git command called it, except in some messages, so
> we'd certainly like to stop pretending that has something to do with
> insn types.

Exactly.  I'll update the commit message.

> Based on what you've said, correcting this cleanly is complicated in
> some places by the inconvenient fact that the sequencer _does_ care
> which top-level git command called it.  (I haven't checked this; I'm
> just taking it on faith from you.)  If we want to let other git
> commands (like "git rebase" or "git sequence") call into the
> sequencer, that sounds like a way bigger problem than any conflict of
> terminology.

There's no need to trust me: just think about the problem.  We fill
out a replay_opts structure to call into the sequencer with- how does
the sequencer know what to do with this hypothetical command string
(say "cherry-pick") on a fresh invocation?  It needs to translate this
into a replay_action at some point, right?  There are atleast three
places where this happens: prepare_revs(), walk_revs_populate_todo(),
and single_pick().

-- Ram