Re: [PATCH v2 02/14] pull: improve default warning
From: Elijah Newren <hidden>
Date: 2020-12-05 18:43:15
On Fri, Dec 4, 2020 at 5:56 PM Felipe Contreras [off-list ref] wrote:
On Fri, Dec 4, 2020 at 6:56 PM Elijah Newren [off-list ref] wrote:quoted
Hi Felipe, On Fri, Dec 4, 2020 at 4:12 PM Felipe Contreras [off-list ref] wrote:quoted
On Fri, Dec 4, 2020 at 5:00 PM Elijah Newren [off-list ref] wrote:quoted
On Thu, Dec 3, 2020 at 10:16 PM Felipe Contreras [off-list ref] wrote:
[...]
quoted
quoted
quoted
quoted
+ "You can squelch this message by running one of the following commands:\n" + "\n" + " git config pull.rebase false # merge (the default strategy)\n"Should this be labelled as the default given the desire to make --ff-only the default? Perhaps I'm jumping ahead and you plan to change that later in this series.That's right. In the previous series which does indeed make "pull.mode=ff-only" the default [1], I do change the warning to specify the future default [2], but in that series the warnings is changed to: The pull was not fast-forward, in the future you will have to choose a merge, or a rebase. To squelch this message and maintain the current behavior, use: git config --global pull.mode merge To squelch this message and adopt the new behavior now, use: git config --global push.mode ff-only Falling back to old style for now (merge). Read "git pull --help" for more information. Since that series didn't get any traction, I decided to only implement step 1: fix the current situation. And later on another series would do step 2: introduce "pull.mode=ff-only" and do the preparations to make it the default.I like this longer plan.Yeah. It's a better plan, just more work for me.quoted
However, on the shorter scale plan...I think the suggestion to use "git pull --no-rebase" makes the current situation worse rather than fixing it.Well, I already said I partly agree with you: in the --ff-only case the suggestion should not be brought forward. But in the "git pull" default case, *today* it's doing a merge. If uttering --merge and thus making the current behavior explicit instead of implicit seems dangerous it's because it is. But not documenting it doesn't make it any less dangerous.
Sounds like we agree that the future should be ff-only-as-default. I also agree with you that the primary problem is the current default behavior, and I'll agree with you that documenting the current default is okay. However, I disagree that your wording here: + "If unsure, run \"git pull --no-rebase\".\n" does anything of the sort. It does not mention that this is the default behavior the users would get if they provided no flags. More importantly, it makes a recommendation...and one that undercuts the point of the message. It makes it feel like the message shouldn't exist at all in any circumstances. I even suspect that adding that sentence may undercut any efforts towards changing the default to ff-only-as-default. While I'm a big fan of most of what you've done in this series, I will object to its merging for as long as this stays. (I definitely don't have veto power or anything close to it, just stating what my opinion is.)