Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: James Liu <james@jamesliu.io>, git@vger.kernel.org
Subject: Re: [PATCH 0/2] advice: add "all" option to disable all hints
Date: Wed, 24 Apr 2024 08:48:10 +0200	[thread overview]
Message-ID: <ZiirKgXQPLmtrwLT@tanuki> (raw)
In-Reply-To: <xmqqo79z2s24.fsf@gitster.g>

[-- Attachment #1: Type: text/plain, Size: 2963 bytes --]

On Tue, Apr 23, 2024 at 11:28:03PM -0700, Junio C Hamano wrote:
> James Liu <james@jamesliu.io> writes:
> 
> > This patch series adds an "all" advice hint type that can be used as a
> > convenience option for disabling all advice hints.
> 
> Hmph.  I thought this was rejected already and not in so distant
> past, but I am not finding a discussion thread in the archive.
> 
> The design to support the advice.* variables to let individual ones
> to be squelched, without allowing "all", is very much deliberate.
> 
> A user may think they are familiar with all the advices already, but
> with "all", they'll never get a chance to learn new ones added after
> they set the configuration.  Looking from the other direction, a new
> advice message is a way for us to tell our users something important
> for them to know.  For example, we may plan to improve a high-level
> Porcelain command so that it will eventually error out when given an
> input that used to be accepted but behaved in a way that newbies
> felt confusing.  In the first step of such a transition, we will
> introduce a new (and hopefully better) way to achieve what the user
> wants to do, and add an advice message to tell the user, when they
> trigger the codepath whose behaviour will change in the future, in
> the old way that will eventually go away.
> 
> Do not close that communication channel on us.

While I agree that it might not be a good idea to set it for our users,
the usecase mentioned by this patch series is scripting. And here I very
much agree with the sentiment that it makes sense to give an easy knob
to disable all advice (disclosure: James is part of the Gitaly team at
GitLab, and that is where this feature comes from, so I am very much
biased).

It has happened multiple times to us that new advices were introduced
that then subsequently caused regressions in Gitaly because the output
of Git commands now looks different. While addressing such breakage is
easy enough to do, it does add up if we have to run Git with every
single advice that we may hit turned off individually.

Now one could say that we shouldn't execute porcelain tools in our
scripts because it is kind of expected that their output may change at
any point in time, and that is certainly true. But the reality is that
there aren't always good plumbing alternatives available.

Furthermore, we are often forced to parse fragile error messages from
such porcelain tools because Git doesn't provide a better way to dissect
errors. Error codes are mostly meaningless and there is no other data
channel available to us than the error message.

These are problems that run deeper than "We want to disable advices",
and we eventually want to address those over time. But it's a long road
to get there, and meanwhile disabling all advice would be something that
helps us make our scripted uses of Git at least a tiny bit more stable.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2024-04-24  6:48 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-24  3:58 [PATCH 0/2] advice: add "all" option to disable all hints James Liu
2024-04-24  3:58 ` [PATCH 1/2] advice: allow advice type to be provided in tests James Liu
2024-04-24  5:28   ` Patrick Steinhardt
2024-04-24  3:58 ` [PATCH 2/2] advice: add "all" option to disable all hints James Liu
2024-04-24  5:29   ` Patrick Steinhardt
2024-04-24  6:28 ` [PATCH 0/2] " Junio C Hamano
2024-04-24  6:48   ` Patrick Steinhardt [this message]
2024-04-24 13:52     ` Phillip Wood
2024-04-24 14:07       ` Patrick Steinhardt
2024-04-24 14:59         ` Junio C Hamano
2024-04-25  6:46           ` Patrick Steinhardt
2024-04-25 16:18             ` Junio C Hamano
2024-04-24 16:14         ` Dragan Simic
2024-04-24 16:21           ` Dragan Simic
2024-04-24 14:31     ` Junio C Hamano
2024-04-25  6:42       ` Patrick Steinhardt
2024-04-24  7:37   ` Dragan Simic
2024-04-29  1:09 ` [PATCH v2 0/1] advice: add --no-advice global option James Liu
2024-04-29  1:09   ` [PATCH v2 1/1] " James Liu
2024-04-29  4:15     ` Dragan Simic
2024-04-29  5:01       ` James Liu
2024-04-29  5:36         ` Dragan Simic
2024-04-29  5:59           ` Dragan Simic
2024-04-29  6:04             ` Eric Sunshine
2024-04-29  6:12               ` Dragan Simic
2024-04-29  6:40         ` Jeff King
2024-04-29  6:55           ` Dragan Simic
2024-04-29 13:50           ` Junio C Hamano
2024-04-30  0:56           ` James Liu
2024-04-29 13:48       ` Junio C Hamano
2024-04-29 17:05     ` Rubén Justo
2024-04-29 17:54       ` Dragan Simic
2024-04-30  1:47   ` [PATCH v3 0/1] " James Liu
2024-04-30  1:47     ` [PATCH v3 1/1] " James Liu
2024-04-30  5:18       ` Patrick Steinhardt
2024-04-30  6:24         ` Dragan Simic
2024-04-30 16:29       ` Junio C Hamano
2024-05-03  7:17     ` [PATCH v4 0/3] advice: add "all" option to disable all hints James Liu
2024-05-03  7:17       ` [PATCH v4 1/3] doc: clean up usage documentation for --no-* opts James Liu
2024-05-03 17:30         ` Junio C Hamano
2024-05-06  1:39           ` James Liu
2024-05-03  7:17       ` [PATCH v4 2/3] doc: add spacing around paginate options James Liu
2024-05-03 14:32         ` Karthik Nayak
2024-05-03 17:36           ` Junio C Hamano
2024-05-03  7:17       ` [PATCH v4 3/3] advice: add --no-advice global option James Liu
2024-05-08  0:40         ` [PATCH v4 4/3] t0018: two small fixes Junio C Hamano
2024-05-03  7:31       ` [PATCH v4 0/3] advice: add "all" option to disable all hints Dragan Simic
2024-05-03 18:00         ` Re* " Junio C Hamano
2024-05-03 19:26           ` Eric Sunshine
2024-05-03 19:48             ` Junio C Hamano
2024-05-03 20:08               ` Junio C Hamano
2024-05-03 21:24                 ` Eric Sunshine
2024-05-05 11:03                   ` Johannes Schindelin
2024-05-06 16:40                     ` Re* " Junio C Hamano
2024-05-07  0:11                       ` Dragan Simic
2024-05-07  0:21                         ` Junio C Hamano
2024-05-07  4:45                           ` Dragan Simic
2024-05-07  0:01                   ` Dragan Simic
2024-05-03 14:35       ` Karthik Nayak
2024-05-05 23:17         ` James Liu
2024-05-03 17:25       ` Junio C Hamano
2024-05-05 23:20         ` James Liu
2024-05-06  1:10           ` James Liu
2024-05-06 16:47             ` Junio C Hamano
2024-05-06 23:08               ` James Liu
2024-05-07  6:23                 ` Karthik Nayak
2024-05-06 16:41           ` Junio C Hamano

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=ZiirKgXQPLmtrwLT@tanuki \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=james@jamesliu.io \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).