git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] transport: no warning if no server wait-for-done
Date: Mon, 09 Aug 2021 10:31:18 -0700	[thread overview]
Message-ID: <xmqqh7fyfrtl.fsf@gitster.g> (raw)
In-Reply-To: <20210806214612.1501980-1-jonathantanmy@google.com> (Jonathan Tan's message of "Fri, 6 Aug 2021 14:46:12 -0700")

Jonathan Tan <jonathantanmy@google.com> writes:

> When the push.negotiate configuration variable was implemented in
> 477673d6f3 ("send-pack: support push negotiation", 2021-05-05), Git was
> taught to print warning messages whenever that variable was set to true
> but push negotiation didn't happen. However, the lack of push
> negotiation is more similar to things like the usage of protocol v2 - in
> which the new feature is desired for performance, but if the feature
> is not there, the user does not intend to take any action - than to
> things like the usage of --filter - in which the new feature is desired
> for a certain outcome, and if the outcome does not happen, there is a
> high chance that the user would need to do something else (in this case,
> for example, reclone with "--depth" if the user needs the disk space).
>
> Therefore, when pushing with push.negotiate set to true, do not warn if
> wait-for-done is not supported for any reason (e.g. if the server is
> using an older version of Git or if protocol v2 is deactivated on the
> server). This is done by using an internal-use-only parameter to "git
> fetch".

Hmph, if this were a hard error, instead of "print warning
messages", the above discussion is entirely reasonable.  But I am
not sure if squelching the warning unconditionally, especially given
that the feature is relatively young, is a good idea.

I suspect that you are trying to hide the "failed" message that may
sound more alarming then it should be from the end users, but if
that is the case, wouldn't it be a better solution to reword the
message to tone it down?

> -	if (finish_command(&child)) {
> -		/*
> -		 * The information that push negotiation provides is useful but
> -		 * not mandatory.
> -		 */
> -		warning(_("push negotiation failed; proceeding anyway with push"));
> -	}

Perhaps like "optional ancestry negotiation failed---pushing
normally" or some phrasing that assures the users that pushing
without negotiation is perfectly normal?

  parent reply	other threads:[~2021-08-09 17:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-06 21:46 [PATCH] transport: no warning if no server wait-for-done Jonathan Tan
2021-08-07  1:31 ` Ævar Arnfjörð Bjarmason
2021-08-16 22:26   ` Jonathan Tan
2021-08-09 17:31 ` Junio C Hamano [this message]
2021-08-09 19:11   ` Jonathan Nieder
2021-08-09 20:04     ` Junio C Hamano
2021-08-16 23:06       ` Jonathan Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh7fyfrtl.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).