Web lists-archives.com

Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges




Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:

>> diff --git a/sequencer.c b/sequencer.c
>> @@ -2932,7 +2966,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
>> -                       strbuf_addf(&buf, "Merge branch '%.*s'",
>> +                       strbuf_addf(&buf, "Merge %s '%.*s'",
>> +                                   to_merge->next ? "branches" : "branch",
>
> Error messages in this file are already localizable. If this text ever
> becomes localizable, then this "sentence lego" could be problematic
> for translators.

I do not think we'd want to localize these default merge messages,
though.

>> @@ -2956,28 +2991,76 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
>> +               cmd.git_cmd = 1;
>> +               argv_array_push(&cmd.args, "merge");
>> +               argv_array_push(&cmd.args, "-s");
>> +               argv_array_push(&cmd.args, "octopus");
>
> argv_array_pushl(&cmd.args, "-s", "octopus", NULL);
>
> which would make it clear that these two arguments must remain
> together, and prevent someone from coming along and inserting a new
> argument between them.

A valid point.  It is OK to break "merge" and "-s octopus" into
separate push invocations, but not "-s" and "octopus".  Or perhaps
push it as a single "--strategy=octopus" argument, which would be
a better approach anyway.

>> +       (GIT_AUTHOR_NAME="Hank" GIT_AUTHOR_EMAIL="hank@sea.world" \
>> +        git merge -m "Tüntenfüsch" two three) &&
>
> Unnecessary subshell?

Right.