Re: [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy

From: Elijah Newren <newren@gmail.com>
Date: Aug 3, 2021, 23:43 UTC
Message-ID: <CABPp-BHzXWTiFukuibf0JXyBLx=6xDLxLU5Xsyra7sHvULOn_g@mail.gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.2108040102220.55@tvgsbejvaqbjf.bet>

On Tue, Aug 3, 2021 at 5:04 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 31 quoted lines

>
> Hi Elijah,
>
> On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  Documentation/git-rebase.txt       |  7 ++++---
> >  Documentation/merge-strategies.txt | 14 ++++++++++++++
> >  2 files changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> > index b4429954480..3e112011c6f 100644
> > --- a/Documentation/git-rebase.txt
> > +++ b/Documentation/git-rebase.txt
> > @@ -340,9 +340,10 @@ See also INCOMPATIBLE OPTIONS below.
> >
> >  -m::
> >  --merge::
> > -     Use merging strategies to rebase.  When the recursive (default) merge
> > -     strategy is used, this allows rebase to be aware of renames on the
> > -     upstream side.  This is the default.
> > +     Use merging strategies to rebase.  When either the `recursive`
> > +     (default) or `ort` merge strategy is used, this allows rebase
> > +     to be aware of renames on the upstream side.  This is the
> > +     default.
>
> Since this now talks about two merge strategies, I think "This is the
> default" needs to specify which of the two strategies is the default.

It does, but I agree the wording is a bit confusing. "This is the default" refers to the fact that "Using merging strategies to rebase" is the default (as opposed to using git-am). We can then list some of the possible merge strategies and which are the default (which we did in the second sentence). However, diving into a discussion about individual strategies might be out of place here. It looks like that was originally mentioned because that allowed rebase to handle renames, but git-am -3 gained that ability well over a decade ago via falling back to the merge machinery. So perhaps we should just simplify this to:

""" Using merging strategies to rebase (this is the default since Git v2.26.0). Note that there are multiple merge strategies available, and specific ones can be picked with the `--strategy` option. """

Show 28 quoted lines

>
> >  +
> >  Note that a rebase merge works by replaying each commit from the working
> >  branch on top of the <upstream> branch.  Because of this, when a merge
> > diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> > index d21dbd1e051..d13d4a29875 100644
> > --- a/Documentation/merge-strategies.txt
> > +++ b/Documentation/merge-strategies.txt
> > @@ -96,6 +96,20 @@ subtree[=<path>];;
> >       is prefixed (or stripped from the beginning) to make the shape of
> >       two trees to match.
> >
> > +ort::
> > +     This is meant as a drop-in replacement for the `recursive`
> > +     algorithm (as reflected in its acronym -- "Ostensibly
> > +     Recursive's Twin"), and will likely replace it in the future.
> > +     It fixes corner cases that the `recursive` strategy handles
> > +     suboptimally, and is significantly faster in large
> > +     repositories -- especially when many renames are involved.
> > ++
> > +The `ort` strategy takes all the same options as `recursive`.
> > +However, it ignores three of those options: `no-renames`,
> > +`patience` and `diff-algorithm`.  It always runs with rename
> > +detection (it handles it much faster than `recursive` does), and
> > +it specifically uses diff-algorithm=histogram.
>
> Probably `diff-algorithm=histogram` should also be enclosed within
> backticks.

I'll fix that up.