git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 02/16] pull: improve default warning

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 7, 2020, 20:55 UTC
Message-ID
<xmqq4kkx9vzx.fsf@gitster.c.googlers.com>
In-Reply-To
<20201205195313.1557473-3-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 40 quoted lines
> 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@gmail.com>
> ---
>  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"));
Show 14 quoted lines
> +		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.

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.

Previous: Felipe ContrerasNext: Felipe Contreras
Message 16 of 33 in “pull: default warning improvements”
  1. 00/16 pull: default warning improvementsFelipe Contreras, Dec 5, 2020
  2. 01/16 doc: pull: explain what is a fast-forwardFelipe Contreras, Dec 5, 2020
  3. Junio C HamanoDec 7, 2020
  4. Felipe ContrerasDec 7, 2020
  5. Elijah NewrenDec 7, 2020
  6. Junio C HamanoDec 7, 2020
  7. Felipe ContrerasDec 8, 2020
  8. Felipe ContrerasDec 8, 2020
  9. Elijah NewrenDec 8, 2020
  10. Felipe ContrerasDec 8, 2020
  11. Felipe ContrerasDec 8, 2020
  12. 03/16 pull: refactor fast-forward checkFelipe Contreras, Dec 5, 2020
  13. 05/16 pull: trivial cleanupFelipe Contreras, Dec 5, 2020
  14. 04/16 pull: cleanup autostash checkFelipe Contreras, Dec 5, 2020
  15. 02/16 pull: improve default warningFelipe Contreras, Dec 5, 2020
  16. Junio C HamanoDec 7, 2020
  17. Felipe ContrerasDec 7, 2020
  18. 16/16 pull: trivial memory fixFelipe Contreras, Dec 5, 2020
  19. 14/16 test: merge-pull-config: trivial cleanupFelipe Contreras, Dec 5, 2020
  20. 06/16 pull: move default warningFelipe Contreras, Dec 5, 2020
  21. 08/16 pull: trivial whitespace style fixFelipe Contreras, Dec 5, 2020
  22. 07/16 pull: display default warning only when non-ffFelipe Contreras, Dec 5, 2020
  23. 09/16 pull: introduce --merge optionFelipe Contreras, Dec 5, 2020
  24. 13/16 pull: add proper error with --ff-onlyFelipe Contreras, Dec 5, 2020
  25. Felipe ContrerasDec 5, 2020
  26. 1/2 pull: add proper error with --ff-onlyFelipe Contreras, Dec 5, 2020
  27. 2/2 tentative: pull: change the semantics of --ff-onlyFelipe Contreras, Dec 5, 2020
  28. 12/16 pull: move configurations fetchesFelipe Contreras, Dec 5, 2020
  29. 15/16 test: pull-options: revert unnecessary changesFelipe Contreras, Dec 5, 2020
  30. 10/16 pull: show warning with --ffFelipe Contreras, Dec 5, 2020
  31. 11/16 rebase: add REBASE_DEFAULTFelipe Contreras, Dec 5, 2020
  32. Junio C HamanoDec 7, 2020
  33. Felipe ContrerasDec 7, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.