Re: [PATCH v4] pull: warn if the user didn't say whether to rebase or to merge

[Junio C Hamano <gitster@pobox.com>]

Alex Henrie <alexhenrie24@gmail.com> writes:

> - Revise warning message based on Junio's feedback
> - Consistently wrap warning lines to 75 characters for easy viewing in
> PO files

Nice to see attention to such a detail ;-)

> - Fix test failures

Ah, OK, hmmm.  

For --quiet test, that wants to ensure that "pull --quiet" does not
say anything, it certainly stops the test from failing if we set the
configuration before executing such a test, but I wonder if that is
in line with the spirit of the feature the test tries to protect in
the first place.  I would imagine those who write "pull --quiet" in
automation would not want to see any non-error message, and because
this is not an error, they do not want to see any output.  Shouldn't
such a use of "pull --quiet" bypass this warning, too?

> ---
>  builtin/pull.c          | 15 +++++++++++++++
>  t/t5521-pull-options.sh |  3 ++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 3e624d1e00..351b933c4d 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -327,6 +327,21 @@ static enum rebase_type config_get_rebase(void)
>  	if (!git_config_get_value("pull.rebase", &value))
>  		return parse_config_rebase("pull.rebase", value, 1);
>  
> +	if (!opt_ff || strcmp(opt_ff, "--ff-only")) {

If we want to squelch the warning under "--quiet", I think we can do
this.

	if (0 < opt_verbosity &&
	    (!opt_ff || strcmp(opt_ff, "--ff-only"))) {

and ...


> +		warning(_("Pulling without specifying how to reconcile divergent branches is\n"
> ...
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index ccde8ba491..6e890ec936 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -8,7 +8,8 @@ test_expect_success 'setup' '
>  	mkdir parent &&
>  	(cd parent && git init &&
>  	 echo one >file && git add file &&
> -	 git commit -m one)
> +	 git commit -m one) &&
> +	git config pull.rebase false

... this change can safely go.

If we agree that "git pull --quiet" should stay quiet in an
environment where the user has been happy with the default choice,
that is.  I am raising this issue to invite others to think about
it, and I am on the fence, but I am leaning towards saying "yes".

>  '

Regardless of what the resolution for "pull --quiet" would be,
shouldn't we have a test for this change to ensure that we do warn
under the condition we should, and that we do not do so when we
should not?

Thanks.