git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 0/4] Beginning of new merge strategy: New API, empty implementation
Date: Mon, 26 Oct 2020 17:52:06 -0700	[thread overview]
Message-ID: <CABPp-BFUS39pmB0S79WfOAiOmeM7gLrvog=b6zYrGRCo_RQr7Q@mail.gmail.com> (raw)
In-Reply-To: <20201026215622.2138981-1-jonathantanmy@google.com>

On Mon, Oct 26, 2020 at 2:56 PM Jonathan Tan <jonathantanmy@google.com> wrote:
>
> > 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.

Yeah, it is kinda ugly.  I was thinking of adding "merge.backend" when
I discovered that "pull.twohead" already existed.  I didn't know if
two configuration knobs doing the same thing was uglier, or using that
old ugly misnomer was.  Maybe we can use the merge.backend name or
something like it once merge-ort is further along.

> >   * 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.

Oh, interesting idea; I hadn't thought of that.  However, It seems
like a bit of a misfit there too -- fast-rebase isn't used in the
testsuite anywhere and I didn't have plans to add it.  My plan was for
fast-rebase to go away soon (perhaps as soon as the merge-ort
implementation lands), probably long before merge-recursive.c goes
away.  fast-rebase exists just to (1) demonstrate the API and (2)
demonstrate some performance benchmarks I used to guide my performance
decisions.  Once merge-ort has fully landed, reason (2) becomes
obsolete.  Reason (1) might still exist in part, but hopefully we can
start updating sequencer.c to use those ideas and then make
fast-rebase fully obsolete.

> 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).

Thanks for taking a look.  :-)

  reply	other threads:[~2020-10-27  0:52 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-21 13:22 [PATCH 0/4] Beginning of new merge strategy: New API, empty implementation Elijah Newren via GitGitGadget
2020-10-21 13:22 ` [PATCH 1/4] merge-ort: barebones API of new merge strategy with " Elijah Newren via GitGitGadget
2020-10-23 18:12   ` Taylor Blau
2020-10-23 19:02     ` Elijah Newren
2020-10-24 10:46   ` Peter Baumann
     [not found]   ` <CAJm9OHczJJyn=Oq2RBGvTit4hedqs6vaYH1gto-z6emo6=n2dw@mail.gmail.com>
     [not found]     ` <CAJm9OHdfxh8SGdteD48eDCA=ihGZmKJD-E67PFhCdFR63RSSTA@mail.gmail.com>
2020-10-24 14:54       ` Elijah Newren
2020-10-21 13:22 ` [PATCH 2/4] merge-ort-wrappers: new convience wrappers to mimic the old merge API Elijah Newren via GitGitGadget
2020-10-21 13:22 ` [PATCH 3/4] fast-rebase: demonstrate merge-ort's API via temporary/hidden command Elijah Newren via GitGitGadget
2020-10-21 13:22 ` [PATCH 4/4] merge,rebase,revert: select ort or recursive by config or environment Elijah Newren via GitGitGadget
2020-10-22  0:16 ` [PATCH 0/4] Beginning of new merge strategy: New API, empty implementation Elijah Newren
2020-10-26 21:56   ` Jonathan Tan
2020-10-27  0:52     ` Elijah Newren [this message]
2020-10-26 16:57 ` [PATCH v2 " Elijah Newren via GitGitGadget
2020-10-26 16:57   ` [PATCH v2 1/4] merge-ort: barebones API of new merge strategy with " Elijah Newren via GitGitGadget
2020-10-26 20:45     ` Junio C Hamano
2020-10-26 21:18       ` Elijah Newren
2020-10-26 22:10         ` Junio C Hamano
2020-10-26 22:28           ` Elijah Newren
2020-10-26 16:57   ` [PATCH v2 2/4] merge-ort-wrappers: new convience wrappers to mimic the old merge API Elijah Newren via GitGitGadget
2020-10-26 16:57   ` [PATCH v2 3/4] fast-rebase: demonstrate merge-ort's API via temporary/hidden command Elijah Newren via GitGitGadget
2020-10-26 16:57   ` [PATCH v2 4/4] merge,rebase,revert: select ort or recursive by config or environment Elijah Newren via GitGitGadget
2020-10-27  2:08   ` [PATCH v3 0/4] Beginning of new merge strategy: New API, empty implementation Elijah Newren via GitGitGadget
2020-10-27  2:08     ` [PATCH v3 1/4] merge-ort: barebones API of new merge strategy with " Elijah Newren via GitGitGadget
2020-10-27  2:33       ` Eric Sunshine
2020-10-27  4:57         ` Elijah Newren
2020-10-27  7:54           ` Eric Sunshine
2020-10-27  2:08     ` [PATCH v3 2/4] merge-ort-wrappers: new convience wrappers to mimic the old merge API Elijah Newren via GitGitGadget
2020-10-27  2:08     ` [PATCH v3 3/4] fast-rebase: demonstrate merge-ort's API via temporary/hidden command Elijah Newren via GitGitGadget
2020-10-27  9:47       ` SZEDER Gábor
2020-10-27  2:08     ` [PATCH v3 4/4] merge,rebase,revert: select ort or recursive by config or environment Elijah Newren via GitGitGadget
2020-10-29 20:32     ` [PATCH v4 0/4] Beginning of new merge strategy: New API, empty implementation Elijah Newren via GitGitGadget
2020-10-29 20:32       ` [PATCH v4 1/4] merge-ort: barebones API of new merge strategy with " Elijah Newren via GitGitGadget
2020-10-29 20:32       ` [PATCH v4 2/4] merge-ort-wrappers: new convience wrappers to mimic the old merge API Elijah Newren via GitGitGadget
2020-10-29 20:32       ` [PATCH v4 3/4] fast-rebase: demonstrate merge-ort's API via new test-tool command Elijah Newren via GitGitGadget
2020-10-29 20:32       ` [PATCH v4 4/4] merge,rebase,revert: select ort or recursive by config or environment Elijah Newren via GitGitGadget
2020-11-02  9:27       ` [PATCH v4 0/4] Beginning of new merge strategy: New API, empty implementation Jacob Keller
2020-11-02 18:52         ` Elijah Newren
2020-11-07  6:09           ` Elijah Newren
2020-11-02 23:45       ` [PATCH v5 " Elijah Newren via GitGitGadget
2020-11-02 23:45         ` [PATCH v5 1/4] merge-ort: barebones API of new merge strategy with " Elijah Newren via GitGitGadget
2020-11-02 23:45         ` [PATCH v5 2/4] merge-ort-wrappers: new convience wrappers to mimic the old merge API Elijah Newren via GitGitGadget
2020-11-02 23:45         ` [PATCH v5 3/4] fast-rebase: demonstrate merge-ort's API via new test-tool command Elijah Newren via GitGitGadget
2020-11-02 23:45         ` [PATCH v5 4/4] merge,rebase,revert: select ort or recursive by config or environment Elijah Newren via GitGitGadget
2020-11-03  1:03         ` [PATCH v5 0/4] Beginning of new merge strategy: New API, empty implementation Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABPp-BFUS39pmB0S79WfOAiOmeM7gLrvog=b6zYrGRCo_RQr7Q@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).