Re: [PATCH v3 4/4] scalar reconfigure: add --maintenance=<mode> option

From: Derrick Stolee <stolee@gmail.com>
Date: May 12, 2025, 18:02 UTC
Message-ID: <82c2c073-3fa6-4023-82d5-bdb772c3790b@gmail.com>
In-Reply-To: <xmqqy0v1sp05.fsf@gitster.g>

On 5/12/2025 1:44 PM, Junio C Hamano wrote:

Show 20 quoted lines

> Derrick Stolee <stolee@gmail.com> writes:
> 
>> My intention was to bend over backwards to prevent a behavior change
>> in the default case. However, I'm coming around to understand that
>> we don't need this background maintenance to be redone every time
>> and can become a no-op by default. (Other new configuration will
>> still happen.)
>>
>> In the case where we're fine changing the default behavior, then
>> the standard --[no-]maintenance option will work, though it is a
>> three-way switch where the lack of its existence means "don't do
>> either mode".
> 
> Ahh, OK.  I misread your intention.
> 
> If it is common for existing users to disable maintenance, perhaps
> by mistake, together with configuration changes that are not quite
> right, perhaps also by mistake, and if they used reconfigure to
> recover from such mistakes, it indeed may make sense to nuke the
> current setting and enable maintenance unconditionally.

The original intent of updating background maintenance in the "reconfigure" command was primarily about updating the schedule, if needed.

The recent bug in the macOS scheduler is a good example of this. Users will need to rerun 'git maintenance start' somewhere in order to get the "correct" schedule. This bug only needs this run for any single repo, which makes the 'scalar reconfigure -a' command a somewhat strange place to get the fix.

> As you suggested in a part of your response I omitted, we can
> annotate <mode> to give hints on the valid choices to help users,
> without changing the default behaviour.  I am personally fine either
> way, as long as we clearly document the reasoning behind our design.

I'll create a new patch on top of the current series version that does this, calling it out as an intentional pattern. It's previously been used by these examples:

 * --fixup=[(amend|reword):]<commit>
 * --diff-filter=[(A|C|D|M|R|T|U|X|B)...[*]]
 * --tool=[g,n,]vimdiff
 * --exclude-hidden=[fetch|receive|uploadpack]

One place where this kind of notation could be helpful, but appears to be absent, is the '-L(<n>:<m>)|(:<method>):<file>' argument for 'git log' and 'git rev-list'. Perhaps this is too dense, though, so it would be better split into '-L<n>:<m>:<file>' and '-L:<method>:<file>'.

Thanks, -Stolee