From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sun, 08 Jan 2012 19:31:10 GMT
Subject: Re: [PATCH 1/6] revert: move replay_action, replay_subcommand to header
Message-ID: <20120108193110.GF1942@burratino>
In-Reply-To: <1326025653-11922-2-git-send-email-artagnon@gmail.com>

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