Re: [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy

From: Elijah Newren <newren@gmail.com>
Date: Aug 4, 2021, 00:37 UTC
Message-ID: <CABPp-BErWux0inUR_FonzVwX0eKxGrjAofo+GYjMNE4FV0qwsQ@mail.gmail.com>
In-Reply-To: <xmqqmtpyrrhe.fsf@gitster.g>

On Tue, Aug 3, 2021 at 6:19 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 12 quoted lines

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > The resolve merge strategy was given prominent positioning in this
> > document, being listed first since it was the default at the time the
> > document was written.  It hasn't been the default since before Git v1.0
> > was released, though.  Move it later in the document, near `octopus` and
> > `ours`.
>
> I think it was listed first because it was written first.

Do you want it to be kept first for that reason as well? I thought it made more sense to cover the default strategy first, but I can move it back if you prefer historical implementation order.

Show 6 quoted lines

> > Further, the wording for "resolve" claimed that it was "considered
> > generally safe and fast", which implies that the other strategies are
> > not.
>
> I do not think it never implied any such thing; it is good to remove
> it, or add the same to all strategies, though.

I am unsure if your double negatives are intentional (not..never)...but I think you're saying it's okay to remove this text.

Show 32 quoted lines

> > diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> > index 5d707e952aa..6b6017e1cc8 100644
> > --- a/Documentation/merge-strategies.txt
> > +++ b/Documentation/merge-strategies.txt
> > @@ -6,13 +6,6 @@ backend 'merge strategies' to be chosen with `-s` option.  Some strategies
> >  can also take their own options, which can be passed by giving `-X<option>`
> >  arguments to `git merge` and/or `git pull`.
> >
> > -resolve::
> > -     This can only resolve two heads (i.e. the current branch
> > -     and another branch you pulled from) using a 3-way merge
> > -     algorithm.  It tries to carefully detect criss-cross
> > -     merge ambiguities and is considered generally safe and
> > -     fast.
> > -
> >  recursive::
> >       This can only resolve two heads using a 3-way merge
> >       algorithm.  When there is more than one common
> > @@ -106,6 +99,13 @@ subtree[=<path>];;
> >       is prefixed (or stripped from the beginning) to make the shape of
> >       two trees to match.
> >
> > +resolve::
> > +     This can only resolve two heads (i.e. the current branch
> > +     and another branch you pulled from) using a 3-way merge
> > +     algorithm.  It tries to carefully detect criss-cross
> > +     merge ambiguities.  It cannot handle renames.  This was
> > +     the default merge algorithm prior to November 2005.
>
> "It does not handle renames"; it wasn't like we wanted to do so and
> cannot---we didn't want to, didn't think it was worth doing, it was
> not part of the design objective to do renames, period.

Right, sorry for the poor wording.  I'll fix it to use your phrase.

> I do not think it is even worth mentioning that it was the default
> in the past.  And I do not think it is worth saying what timeframe
> the recursive was the default, either.

I don't feel strongly about it and I can strike it. My reasoning was that providing historical timeframes might help them repeat or recreate old merges in their own repositories.

However, I agree that the more likely bits of information that will help users select the strategy they want are the capabilities (e.g. renames, criss-cross merges, number of heads, etc.).