From: Phillip Wood Date: Wed, 24 Apr 2024 13:52:39 GMT Subject: Re: [PATCH 0/2] advice: add "all" option to disable all hints Message-ID: <7d1d2892-d486-4e91-b100-e44c1d76893c@gmail.com> In-Reply-To: Hi Patrick On 24/04/2024 07:48, Patrick Steinhardt wrote: > On Tue, Apr 23, 2024 at 11:28:03PM -0700, Junio C Hamano wrote: >> 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). Maybe an environment variable would be a better fit for turning advice off in scripts? > 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. I'm sure you've considered these suggestions but (a) would it be possible for Gitaly to filter out lines beginning with "hint: " when it captures the output of commands and (b) would it be possible to have a script that parses advice_setting in advice.c to find the list of advice names so Gitaly can disable them? I think (a) would still leave some advice output from code that uses advice_enabled() rather than advise_if_enabled() but it should get rid of most of the advice messages. Best Wishes Phillip > 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