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

Commit Message

Elijah Newren Aug. 3, 2021, 3:35 p.m. UTC

From: Elijah Newren <[e-mail]>

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

Further, the wording for "resolve" claimed that it was "considered
generally safe and fast", which implies that the other strategies are
not.  While such an implication may have been true in 2005 when written,
it may well be that `ort` is faster today (since it does not need to
recurse into all directories).  Also, since `resolve` was the default
for less than a year while `recursive` has been the default for a decade
and a half, I think `recursive` is more battle-tested than `resolve` is.
Just strike this extraneous phrase.

Also, provide some quick historical context that may help users
understand its purpose and place in the list of merge strategies.

Signed-off-by: Elijah Newren <[e-mail]>
---
 Documentation/merge-strategies.txt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Junio C Hamano Aug. 4, 2021, 12:19 a.m. UTC | #1

"Elijah Newren via GitGitGadget" <[e-mail]> writes:

> From: Elijah Newren <[e-mail]>
>
> 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.

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

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

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.

>  octopus::
>  	This resolves cases with more than two heads, but refuses to do
>  	a complex merge that needs manual resolution.  It is

Elijah Newren Aug. 4, 2021, 12:37 a.m. UTC | #2

On Tue, Aug 3, 2021 at 6:19 PM Junio C Hamano <[e-mail]> wrote:
>
> "Elijah Newren via GitGitGadget" <[e-mail]> writes:
>
> > From: Elijah Newren <[e-mail]>
> >
> > 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.

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

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

Junio C Hamano Aug. 4, 2021, 2:01 a.m. UTC | #3

Elijah Newren <[e-mail]> writes:

> On Tue, Aug 3, 2021 at 6:19 PM Junio C Hamano <[e-mail]> wrote:
>>
>> "Elijah Newren via GitGitGadget" <[e-mail]> writes:
>>
>> > From: Elijah Newren <[e-mail]>
>> >
>> > 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.

No, I was only correcting the guess described in the log message.

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

Yes.

Patch

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.
+
 octopus::
 	This resolves cases with more than two heads, but refuses to do
 	a complex merge that needs manual resolution.  It is