Subject: Re: [PATCH 1/6] revert: move replay_action, replay_subcommand to header

From: Jonathan Nieder <[email-masked]>
Date: Sun, 8 Jan 2012 13:31:10 -0600

Ramkumar Ramachandra wrote:

> Our plan to build a sequencer involves leaving the revert builtin with
> just argument parsing work.  Since the enums replay_action and
> replay_subcommand have nothing to do with this argument parsing, move
> them to sequencer.h in advance.
>
> "REVERT" and "CHERRY_PICK" are unsuitable variable names for exposing
> publicly, as their purpose is unclear in the global context: rename
> them to "REPLAY_REVERT" and "REPLAY_PICK" respectively.

My first reaction: this probably would be more self-explanatory if
squashed with the patch the moves the rest of the code to sequencer.[ch].

Second reaction: ah, but the s/REVERT/REPLAY_REVERT/ and
s/CHERRY_PICK/REPLAY_PICK/ changes would be lost in the noise of the move.
Maybe it would be possible to make those changes but keep the enum in
builtin/revert.c, explaining that this is a preparatory step and they
will be moving in a few moments?

Hope that helps,
Jonathan