Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}

[Avery Pennarun <apenwarr@gmail.com>]

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