Re: [PATCH 0/4] Beginning of new merge strategy: New API, empty implementation

From: Jonathan Tan <jonathantanmy@google.com>
To: newren@gmail.com
Cc: gitgitgadget@gmail.com, git@vger.kernel.org,
	Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH 0/4] Beginning of new merge strategy: New API, empty implementation
Date: Mon, 26 Oct 2020 14:56:22 -0700	[thread overview]
Message-ID: <20201026215622.2138981-1-jonathantanmy@google.com> (raw)
In-Reply-To: <CABPp-BEGo8qfaoz6vUs+ktvmNUZdiHjwda468Qkh5uaD-wAd4w@mail.gmail.com>

> Hi,
> 
> On Wed, Oct 21, 2020 at 6:22 AM Elijah Newren via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > In this series, I try to show the new merge API I have developed in
> > merge-ort and show how it differs from that provided by merge-recursive. I
> > do this in four steps, each corresponding to a patch:
> 
> I should probably call out that even if folks don't have time to
> review patches, I'm particularly interested in opinions on the
> following two questions:
>   * Are the "pull.twohead" and "GIT_TEST_MERGE_ALGORITHM" names in
> patch 4 good/bad/ugly?  (especially the mapping from "pull" to revert,
> cherry-pick, rebase, and merge?)

I think GIT_TEST_MERGE_ALGORITHM is fine. I think extending
"pull.twohead" is ugly - perhaps we could get away with not doing
anything about it for now, and once this feature is ready, we could add
a new config parameter specially for this.

>   * Is it too weird to have a temporary/hidden builtin, in patch 3?
> If so, what is a good alternative?

An alternative is to implement it as part of test-tool - see t/helper/
for more information.

I'm not very familiar with the merge code, but overall this looks like a
good, clear design. Thanks especially for showing how this code can be
used (in patch 3) and a comparison to the old API (in patch 2).