From: Jeff Hostetler <git@jeffhostetler.com>
Date: Mon, 21 Mar 2022 18:47:20 GMT
Subject: Re: [PATCH 02/16] update-index: convert advise() messages back to warning()
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:
> "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