Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
> [...]
>> -static const char *action_name(const struct replay_opts *opts)
>> +static const char *command_name(struct replay_opts *opts)
>
> This part does a similar renaming, and drops a const while at it for
> no intelligible reason.
Carried over from the previous iteration- sorry I forgot to fix this.
> [...]
>> @@ -142,7 +147,7 @@ static void verify_opt_mutually_compatible(const char *me, ...)
>> static void parse_args(int argc, const char **argv, struct replay_opts *opts)
>> {
>> const char * const * usage_str = revert_or_cherry_pick_usage(opts);
>> - const char *me = action_name(opts);
>> + const char *me = command_name(opts);
>
> The rest is stuff like this, which follows from the first part.
>
> Stepping back, I think the idea is that "enum replay_action" is not a
> good way to identify the command name in error messages like
>
> fatal: cherry-pick: --abort cannot be used with --continue
>
> So you introduce a _new_ enum to represent the command name. Why not
> just use a string, so commands using the nice and generic sequencer
> library do not have to register themselves in a global callers list to
> use it?
Fine; I'm sold on the string idea. Also, I figured it would be
easier to explain the changes if I change this enum to a string -- I
should probably use "ease of explaining changes" as a stronger
criterion in the future when I have two competing implementations in
mind.
Thanks.
-- Ram
--
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