On Mon, Dec 7, 2020 at 2:55 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>
> > We want to:
> >
> > 1. Be clear about what "specifying" means; merge, rebase, or
> > fast-forward.
> > 2. Mention a direct shortcut for people that just want to get on with
> > their lives: git pull --no-rebase.
> > 3. Have a quick reference for users to understand what this
> > "fast-forward" business means.
> >
> > This patch does all three.
> >
> > Thanks to the previous patch now "git pull --help" explains what a
> > fast-forward is, and a further patch changes --no-rebase to --merge so
> > it's actually user friendly.
> >
> > Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> > ---
> > builtin/pull.c | 25 +++++++++++++------------
> > 1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/builtin/pull.c b/builtin/pull.c
> > index 1034372f8b..d71344fe28 100644
> > --- a/builtin/pull.c
> > +++ b/builtin/pull.c
> > @@ -345,18 +345,19 @@ static enum rebase_type config_get_rebase(void)
> > return parse_config_rebase("pull.rebase", value, 1);
> >
> > if (opt_verbosity >= 0 && !opt_ff) {
> > - advise(_("Pulling without specifying how to reconcile divergent branches is\n"
> > - "discouraged. You can squelch this message by running one of the following\n"
> > - "commands sometime before your next pull:\n"
> > - "\n"
> > - " git config pull.rebase false # merge (the default strategy)\n"
> > - " git config pull.rebase true # rebase\n"
> > - " git config pull.ff only # fast-forward only\n"
> > - "\n"
> > - "You can replace \"git config\" with \"git config --global\" to set a default\n"
> > - "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
> > - "or --ff-only on the command line to override the configured default per\n"
> > - "invocation.\n"));
>
>
> > + advise(_("Pulling without specifying how to reconcile divergent branches is discouraged;\n"
> > + "you need to specify if you want a merge, a rebase, or a fast-forward.\n"
> > + "You can squelch this message by running one of the following commands:\n"
> > + "\n"
> > + " git config pull.rebase false # merge (the default strategy)\n"
> > + " git config pull.rebase true # rebase\n"
> > + " git config pull.ff only # fast-forward only\n"
> > + "\n"
> > + "You can replace \"git config\" with \"git config --global\" to set a default\n"
> > + "preference for all repositories.\n"
> > + "If unsure, run \"git pull --no-rebase\".\n"
> > + "Read \"git pull --help\" for more information."
> > + ));
> > }
>
> It is an improvement to say what they can do from the command line
> in order to get out of the current situation, before giving them a
> configuration advice.
>
> But the exact instruction for the command line in the original seems
> to have been lost during the rewrite, which I think makes the "what
> to do now" advice less valuable than it could be.
In my opinion we should not dump the entire contents of the manpage in
a warning. The warning should be as brief as possible while giving
these:
1. A description of what's going on
2. A suggested way to move forward
3. A quick way to opt-out
4. A way to find more information
That's it.
Of course when it comes to succinctness other developers tend to
disagree with me, but that's my opinion.
> I personally think it is backwards to update this message before
> fixing the condition under which it triggers (I think by now
> everybody involved in the thread understands that we do not want to
> give this advise that does not stop to behave as-is---it should be
> made not to trigger if we know the history would fast-forward, and
> when it triggers it should stop the operation). It may appear OK as
> long as we get the right end-state, but because this message must be
> rewritten when the triggering condition changes (i.e. when we stop
> giving this message when the history fast-forwards, there is no
> point in offering the fast-forward-only as a viable option), it
> seems to me a needless detour.
Except it's not necessary to rewrite it further, not in step 1.
Maybe it will be clearer when I send all the patches.
--
Felipe Contreras
