Re: [PATCH 1/8] git-merge-file --ours, --theirs

[Avery Pennarun <apenwarr@gmail.com>]

On Thu, Nov 26, 2009 at 1:17 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Except for parse-optification, this one is more or less a verbatim copy of
> my patch, and I think I probably deserve an in-body "From: " line for this
> [PATCH 1/8], [PATCH 6/8] and [PATCH 8/8] to take the full authorship of
> them.
[...]
> Imagine that Avery were an area expert (the subsystem maintainer) on "git
> merge" and downwards, and somebody who did not know that "merge" has
> already been rewritten in C, nor some parts of the system have been
> rewritten to use parse-options, submitted a patch series for review and
> Avery is helping to polish it up [*1*].

I'm quite open to doing this however you want; I definitely consider
it your patch series.  My main measurable contribution is just the
unit tests that I wrote.

However, when thinking about this, I wasn't worried so much about the
correct placement of credit as of discredit.  The merge code has
changed sufficiently since you wrote this patch series that every one
of them required quite a lot of conflict resolution.  Most of the
conflicts were pretty obvious how to resolve, but it was tedious and
error prone, and there's a reasonably high probability that I screwed
up something while doing so.

I imagined what people would expect to see when they do 'git blame' to
explain the source of a problem.  If they see your name, you might be
blamed for my errors; if they see my name with a "based on a patch by
Junio" in the changelog, then I would be (probably correctly) blamed
for the errors, while you can retain credit for the success.

Mostly, however, I didn't want to be sending out patches in your name
that weren't actually done by you.  If you'd like me to do so,
however, then I will :)

>> +/* merge favor modes */
>> +#define XDL_MERGE_FAVOR_OURS 0x0010
>> +#define XDL_MERGE_FAVOR_THEIRS 0x0020
>> +#define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)
>
> This is a bad change.  It forces the high-level layer of the resulting
> code to be aware that the favor bits are shifted by 4 and it is different
> from what the low-level layer expects.  If I were porting it to
> parse-options, I would have kept OURS = 1 and THEIRS = 2 as the original
> patch, [...]

Ouch, yes, that wasn't very clear thinking on my part.  I meant for
XDL_MERGE_FAVOR(flags) to return either XDL_MERGE_FAVOR_OURS or
XDL_MERGE_FAVOR_THEIRS, but clearly it doesn't.  Will fix.

Avery