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

Re: [PATCH v2 02/14] pull: improve default warning

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 7, 2020, 19:53 UTC
Message-ID
<xmqqlfe99yvy.fsf@gitster.c.googlers.com>
In-Reply-To
<CAMP44s2nmVnXiBA8S=vHBZznuRNKKe=xGOEBJ80MYhA_XCqNkg@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> They start by saying the same thing. But one errors out and says the
> user must choose, and the other warns that in the future the user must
> choose.

Then I do not see the point in giving the warning---even in the future they do not have to choose as long as they are merely following along.

Show 14 quoted lines
>> > Just to put this series in context: it's only part 1; it does not
>> > introduce pull.mode, and it doesn't make --ff-only the default.
>>
>> I'd view the "in a non-fast-forward situation, the warning kicks in
>> to those who haven't chosen between merge and rebase (i.e. no
>> pull.rebase set to either true or false, and pull.ff not set to
>> only), which is a bit more gentle than the current situtation" a
>> good stopping point.  That state is already making ff-only the
>> default for unconfigured users, or you can view it as shipping "git
>> pull" in a shape that has the more dangerous half of its feature
>> disabled to avoid hurting users.  So I am not sure why you keep
>> saying you do not have --ff-only as the default.
>
> The warning doesn't make the pull fail, ff-only does.

Then probably you are giving an error and a warning at a wrong place.

 - When history fast-forwards, and the user hasn't chosen between
   rebase or merge, there is no need to give any warning.  Just
   succeed by fast-forwarding.
 - When history does not fast-forward and the user hasn't chosen
   between rebase or merge, whether pull.ff is set to "only" or not,
   we should fail and the error message can instruct the user to
   choose between rebase and merge; there is no "ff-only" option
   that is useful in the situation.

And that essentially makes the "ff-only" mode the safe default that castrates one half of the feature (the more dangerous half) of "git pull". Why do we make it more complicated than that by warning that the user must choose in the future? They will see an error tell them that when they start pulling while on their own work, and I do not see a need to bother them before that point.

Previous: Felipe ContrerasNext: Felipe Contreras
Message 25 of 79 in “pull: default warning improvements”
  1. 00/14 pull: default warning improvementsFelipe Contreras, Dec 4, 2020
  2. 03/14 pull: refactor fast-forward checkFelipe Contreras, Dec 4, 2020
  3. 04/14 pull: cleanup autostash checkFelipe Contreras, Dec 4, 2020
  4. Elijah NewrenDec 4, 2020
  5. Felipe ContrerasDec 5, 2020
  6. Elijah NewrenDec 5, 2020
  7. 01/14 doc: pull: explain what is a fast-forwardFelipe Contreras, Dec 4, 2020
  8. Elijah NewrenDec 4, 2020
  9. Jacob KellerDec 5, 2020
  10. 02/14 pull: improve default warningFelipe Contreras, Dec 4, 2020
  11. Elijah NewrenDec 4, 2020
  12. Felipe ContrerasDec 5, 2020
  13. Elijah NewrenDec 5, 2020
  14. Felipe ContrerasDec 5, 2020
  15. Chris TorekDec 5, 2020
  16. Felipe ContrerasDec 5, 2020
  17. Elijah NewrenDec 5, 2020
  18. Felipe ContrerasDec 5, 2020
  19. Elijah NewrenDec 6, 2020
  20. Felipe ContrerasDec 6, 2020
  21. Junio C HamanoDec 7, 2020
  22. Felipe ContrerasDec 7, 2020
  23. Junio C HamanoDec 7, 2020
  24. Felipe ContrerasDec 7, 2020
  25. Junio C HamanoDec 7, 2020
  26. Felipe ContrerasDec 7, 2020
  27. Jacob KellerDec 7, 2020
  28. Junio C HamanoDec 8, 2020
  29. Felipe ContrerasDec 8, 2020
  30. Junio C HamanoDec 8, 2020
  31. Felipe ContrerasDec 9, 2020
  32. Elijah NewrenDec 9, 2020
  33. Felipe ContrerasDec 10, 2020
  34. Junio C HamanoDec 10, 2020
  35. Felipe ContrerasDec 10, 2020
  36. Felipe ContrerasDec 10, 2020
  37. Junio C HamanoDec 11, 2020
  38. Felipe ContrerasDec 11, 2020
  39. Junio C HamanoDec 14, 2020
  40. Felipe ContrerasDec 14, 2020
  41. Junio C HamanoDec 14, 2020
  42. Felipe ContrerasDec 15, 2020
  43. Jeff KingDec 15, 2020
  44. Felipe ContrerasDec 15, 2020
  45. Junio C HamanoDec 15, 2020
  46. Felipe ContrerasDec 15, 2020
  47. Felipe ContrerasDec 7, 2020
  48. 06/14 pull: move default warningFelipe Contreras, Dec 4, 2020
  49. Elijah NewrenDec 4, 2020
  50. Felipe ContrerasDec 4, 2020
  51. 05/14 pull: trivial cleanupFelipe Contreras, Dec 4, 2020
  52. Elijah NewrenDec 4, 2020
  53. Felipe ContrerasDec 5, 2020
  54. 08/14 pull: trivial whitespace style fixFelipe Contreras, Dec 4, 2020
  55. 10/14 pull: add proper error with --ff-onlyFelipe Contreras, Dec 4, 2020
  56. Elijah NewrenDec 4, 2020
  57. Felipe ContrerasDec 5, 2020
  58. 12/14 pull: show warning with --ffFelipe Contreras, Dec 4, 2020
  59. Elijah NewrenDec 4, 2020
  60. Felipe ContrerasDec 5, 2020
  61. 14/14 test: pull-options: revert unnecessary changesFelipe Contreras, Dec 4, 2020
  62. Elijah NewrenDec 4, 2020
  63. Felipe ContrerasDec 5, 2020
  64. 13/14 test: merge-pull-config: trivial cleanupFelipe Contreras, Dec 4, 2020
  65. Elijah NewrenDec 4, 2020
  66. 11/14 tentative: pull: change the semantics of --ff-onlyFelipe Contreras, Dec 4, 2020
  67. Elijah NewrenDec 4, 2020
  68. Felipe ContrerasDec 5, 2020
  69. experiment: pull: change --ff-only and default modeFelipe Contreras, Dec 5, 2020
  70. Elijah NewrenDec 5, 2020
  71. Felipe ContrerasDec 5, 2020
  72. Junio C HamanoDec 7, 2020
  73. Felipe ContrerasDec 5, 2020
  74. 07/14 pull: display default warning only when non-ffFelipe Contreras, Dec 4, 2020
  75. Elijah NewrenDec 4, 2020
  76. Felipe ContrerasDec 5, 2020
  77. 09/14 pull: introduce --merge optionFelipe Contreras, Dec 4, 2020
  78. Elijah NewrenDec 4, 2020
  79. Felipe ContrerasDec 5, 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.