Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jan 11, 2012, 09:19 UTC
Message-ID: <CALkWK0=o+KkV08G9JuTaG8Vtb-AzHQVMQPzMy8td_iBVnGX4Dg@mail.gmail.com>
In-Reply-To: <20120111054954.GB13507@burratino>

Jonathan Nieder wrote:

Show 9 quoted lines

> 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.

Show 7 quoted lines

> 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