Re: [PATCH v2 4/4] scalar reconfigure: add --no-maintenance option

From: Derrick Stolee <stolee@gmail.com>
Date: May 6, 2025, 18:00 UTC
Message-ID: <3e307bf1-52ef-4280-bada-5cc50cc7a817@gmail.com>
In-Reply-To: <xmqqselin30f.fsf@gitster.g>

On 5/5/2025 5:47 PM, Junio C Hamano wrote:

Show 41 quoted lines

> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> When users want to enable the latest and greatest configuration options
>> recommended by Scalar after a Git upgrade, 'scalar reconfigure --all' is
>> a great option that iterates over all repos in the multi-valued
>> 'scalar.repos' config key.
>>
>> However, this feature previously forced users to enable background
>> maintenance. In some environments this is not preferred.
>>
>> Add a new --[no-]maintenance option to 'scalar reconfigure' that avoids
>> running 'git maintenance start' on these enlistments.
> 
> It makes sense for --maintenance option to be between enable and
> disable when registering a new directory to the system, and when
> cloning somebody else's repository that causes a new directory to be
> created and enlisting the resulting new directory to the system.
> 
> But wouldn't users want "leave maintenance-enrollment status alone"
> option when reconfiguring an existing already enlisted directory?
> 
> As written, the design easily allows enabling of maintenance as part
> of reconfiguring, but disabling cannot be done the same way
> (i.e. individual enlistments need to be visited and their
> maintenance disabled manually).
> 
> IOW, it is a bit counter-intuitive
> 
>> +--[no-]maintenance::
>> +	By default, Scalar configures the enlistment to use Git's
>> +	background maintenance feature. Use the `--no-maintenance` to skip
>> +	this configuration and leave the repositories in whatever state is
>> +	currently configured.
> 
> that for clone and register, --maintenance means "enable" and
> "--no-maintenance" means "disable", but when reconfiguring an
> already registered directory, it would be natural to expect that
> "--no-maintenance" would explicitly tell the command to disable
> scheduled maintenance.

I can see how this command is different from the other two, and thus a three-way flipper can actually result in three different behaviors:

Show 13 quoted lines

> A 3-way approach would make this part something like ...
> 
> 	switch (maintenance) {
> 	default:	BUG("..."); break;
> 	case ENABLE:	res = toggle_maintenance(1); break;
> 	case DISABLE:	res = toggle_maintenance(0); break;
> 	case ASIS:	res = 0; break;
> 	}
> 	if (res >= 0)
> 		succeeded = 1;
> 
> ... which would allow people to easily say "leave the existing
> maintenance state alone".

This does mean that we'd need to have a different toggle from the typical OPT_BOOL().

What do you think about something of the form --maintenance=<option> where <option> is one of these:

 * "enable" (default) runs 'git maintenance start'
 * "disable" runs 'git maintenance unregister'
 * "keep" does not mess with maintenance config.

Does this sort of option seem to make sense? I'll wait to see if any further adjustments are recommended before I start rolling a new version.

Thanks, -Stolee