From: Avery Pennarun <apenwarr@gmail.com>
Date: Thu, 26 Nov 2009 22:05:23 GMT
Subject: Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}
Message-ID: <32541b130911261405q6564d8f2o30b7d7fd6f708d05@mail.gmail.com>
In-Reply-To: <7vr5rlerqf.fsf@alter.siamese.dyndns.org>

On Thu, Nov 26, 2009 at 1:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
>  - The original series was done over a few weeks in 'pu' and this
>   intermediate step was done before a better alternative of not using
>   these two extra merge strategies were discovered ("...may have been an
>   easy way to experiment, but we should bite the bullet", in the next
>   patch).
>
>   As the second round to seriously polish the series for inclusion, it
>   would make much more sense to squash this with the next patch to erase
>   this failed approach that has already been shown as clearly inferiour.

ok.

>  - I think we should avoid adding the extra argument to ll_merge_fn() by
>   combining virtual_ancestor and favor into one "flags" parameter.  If
>   you do so, we do not have to change the callsites again next time we
>   need to add new optional features that needs only a few bits.
>
>   I vaguely recall that I did the counterpart of this patch that way
>   exactly for the above reason, but it is more than a year ago, so maybe
>   I didn't do it that way.

You did do that, in fact, but I had to redo a bunch of the flag stuff
anyway since a few other flags had been added in the meantime.

I actually tried it both ways (with and without an extra parameter),
but I observed that:

- There are more lines of code (and more confusion) if you use an
all-in-one flags vs. what I did.

- Several functions have the same signature with all-in-one flags vs.
their current boolean parameter, so the code would compile (and then
subtly not work) if I forgot to modify a particular function.

- When we go to add a third flag parameter, it wouldn't be any harder
to join them together at that time, and because it would *again*
modify the function signatures (from two flag params back down to
one), the compiler would *again* be able to catch any functions we
forgot to adjust.

If you think this logic doesn't work, I can redo it with all-in-one
flags as you request.

Avery