Re: [PATCH 1/8] git-merge-file --ours, --theirs
- From
Avery Pennarun <apenwarr@gmail.com>
- Date
- Nov 26, 2009, 21:55 UTC
- Message-ID
- <32541b130911261355y2900b0cbtbf081c93c8fb10d6@mail.gmail.com>
- In-Reply-To
- <7vy6ltdd2l.fsf@alter.siamese.dyndns.org>
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.
[...]
Show 5 quoted lines
> 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 :)
Show 10 quoted lines
>> +/* 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