Thread (3 messages) 3 messages, 3 authors, 2020-12-07

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

From: Jacob Keller <hidden>
Date: 2020-12-07 23:30:58

On Mon, Dec 7, 2020 at 11:53 AM Junio C Hamano [off-list ref] wrote:
Felipe Contreras [off-list ref] writes:
quoted
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.
I think the key point is that this "in the future" is referring to "a
future version of git will make this an error"

This might be better if it said something like "The pull was not
fast-forward. In a future version of git you will need to specify
whether to merge or rebase, using pull.mode"

or something similar. In theory, this warning will go away once that
future version of git changes so that pull.mode defaults to ff-only.

The difference being that a warning will allow the command to continue
doing the default of today (merging), where as an error will stop the
command essentially just after the fetch portion finishes, without
changing the branch.

Thanks,
Jake
quoted
quoted
quoted
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.
Correct.
 - 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.
Yes, I understand that this is the destination Felipe wants us to end up at.
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.
The warning would be in an earlier version of git before that is the
default. The warning is for a transition period between today when we
merge by default (with a warning) and when we fail with an error
without completing the pull. Once the default is the ff-only mode,
then the warning is no longer necessary, as we will get an error
indicating that you must choose.

Thanks,
Jake
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help