Re: [PATCH 02/16] update-index: convert advise() messages back to warning()

From: Jeff Hostetler <git@jeffhostetler.com>
Date: Mar 21, 2022, 18:47 UTC
Message-ID: <3ba50173-8739-3741-7226-0299ea941e00@jeffhostetler.com>
In-Reply-To: <xmqq5yoht6an.fsf@gitster.g>

On 3/14/22 2:08 AM, Junio C Hamano wrote:

Show 11 quoted lines

> "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>>
>> fixup! update-index: convert fsmonitor warnings to advise
> 
> Same comment as 01/16 applies here.  "convert ... back to ..." in
> the title refers to the fact that builtin-fsmonitor-part2 topic
> turned warning() into advise() without a good justification, I
> think.  Flipping and flopping between warning and advise, without
> giving any justification going either direction, is not a good move.

Sorry for not including the backstory.

Ben wrote the original warning message back in 2017.

In my original version of part 2 I added a more detailed warning message because of the new `core.useBuiltinFSMonitor` config settings and how it interacted with `core.fsmonitor`. And we talked about making that longer message advise rather than a warning. So I changed them to advise().

But then we decided to remove `core.useBuiltinFSMonitor` and overload `core.fsmonitor` to take a boolean, so the original text of the message was sufficient and correct. So to minimize the diff, I reverted the text change and kept the change from warning() to advise().

But then there were comments from AEvar on either changing all of the nearby warning() calls to advise() *and* to change them to use the `advise_type` enum. That seemed like a large/disruptive change at this point.

Also, I wasn't really sure of the need for Ben's original warning message, so I'd rather leave it as is than expand the scope.

So I basically reverted the change for this series. If (in a later series) we want to revisit all of the warnings in update-index.c, then we can think about this.

Jeff