Alex Henrie <alexhenrie24@xxxxxxxxx> 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.
