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

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jan 11, 2012, 10:11 UTC
Message-ID: <CALkWK0mv2jzmDA==pJg5R4jH0yxo=OopYM_WzAWusiffnb+4HQ@mail.gmail.com>
In-Reply-To: <20120111095236.GB31670@burratino>

Jonathan Nieder wrote:

Show 13 quoted lines

> Perhaps cherry-pick and revert should be different values for
> replay_subcommand, to avoid conflating the mechanics and the command
> name?  Resulting in something like this:
>
>        enum replay_subcommand {
>                REPLAY_PICK_REVISIONS,
>                REPLAY_REVERT_REVISIONS,
>                REPLAY_EDIT_SEQUENCE,
>                REPLAY_REMOVE_STATE,
>                REPLAY_CONTINUE,
>                REPLAY_SKIP,
>                REPLAY_ROLLBACK
>        };

We'd be prematurely locking ourselves into a design where we can't tell which top-level command issued the continue/ abort -- this means that there's no way to deny a 'git rebase --continue' from running after a 'git cherry-pick' conflicts (assuming that rebase is implemented in terms of the sequencer ofcourse). Even if that specific objection isn't to your taste, I'm not comfortable about painting ourselves into such a tight corner so early on. My sincere suggestion is to procrastinate the problem until we have a tighter usecase (a new top-level command or action added, for instance). I don't think we have to worry about preserving backward compatibility in the sequencer API?

-- Ram