Re: [PATCH 0/2] scalar: add --no-maintenance option

From: Junio C Hamano <gitster@pobox.com>
Date: May 1, 2025, 16:38 UTC
Message-ID: <xmqqh624xp4j.fsf@gitster.g>
In-Reply-To: <d3659baa-054e-4a31-a851-5471968aed07@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

> Is the right solution to move the toggle_maintenance() out of
> register_dir()? If this is the only way we plan to customize the
> config, then yes. Otherwise, the second or third customization will
> start to lead to copied logic through these three locations.

It is mostly philosophical, I think, but I actually think the callers that are allowed to be different is a good thing. The callers can pass different parameters to register_dir(), but the distinction between these different callers would become more subtle and not immediately obvious to readers. With an explicit call to toggle_maintenance() at each callsite, it becomes more obvious to see who sets up the maintenance job and how.

If this is and will stay the only way, I would not care too much either way, but if we are planning to extend, then I would say that it is more important to allow callers to be more explicit.

Besides, you'd need to call toggle_maintenance() to disable it in a caller outside register_dir(), so it is not like you can hide the tweaks from readers of the code and make it appear to be simpler. They need to be aware of what goes on anyway.