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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html