From: Karthik Nayak <karthik.188@gmail.com>
Date: Thu, 06 Nov 2025 13:07:23 GMT
Subject: Re: [PATCH v3 5/5] maintenance: add 'is-needed' subcommand
Message-ID: <CAOLa=ZS9J9SfMFp7+dmue=isJrpFSbTU7z8TCShOb36XdB8Y_Q@mail.gmail.com>
In-Reply-To: <aQyOZ0e6HO0_77Au@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Nov 06, 2025 at 09:22:34AM +0100, Karthik Nayak wrote:
>> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc
>> index 540b5cf68b..37939510d4 100644
>> --- a/Documentation/git-maintenance.adoc
>> +++ b/Documentation/git-maintenance.adoc
>> @@ -84,6 +85,16 @@ The `unregister` subcommand will report an error if the current repository
>>  is not already registered. Use the `--force` option to return success even
>>  when the current repository is not registered.
>>
>> +is-needed::
>> +    Check whether maintenance needs to be run without actually running it.
>> +    Exits with a 0 status code if maintenance needs to be run, 1 otherwise.
>> +    Ideally used with the '--auto' flag.
>> ++
>> +If one or more `--task` options	are specified, then those tasks are checked
>
> I spoke too soon, forgot that there's one more patch :) s/\t/ /

Weird, not sure how that happened, good catch.

>
>> +in that order. Otherwise, the tasks are determined by which
>> +`maintenance.<task>.enabled` config options are true. By default, only
>> +`maintenance.gc.enabled` is true.
>
> This could use a pointer to "maintenance.strategy", but I see that you
> took this explanation from the "run" subcommand. I think this is good
> enough for now.

Yeah, that's what I went with, so I'll leave it as is :)

[snip]

>> +	if (opts.auto_flag) {
>> +		for (size_t i = 0; i < opts.tasks_nr; i++) {
>> +			if (tasks[opts.tasks[i]].auto_condition &&
>> +			    tasks[opts.tasks[i]].auto_condition(&cfg)) {
>> +				is_needed = true;
>> +				break;
>> +			}
>> +		}
>> +	} else {
>> +		/* When not using --auto, we should always require maintenance. */
>
> Nit: we might add a TODO comment here.
>
>     /*
>      * When not using --auto we always require maintenance right now.
>      *
>      * TODO: this certainly is too eager, as some maintenance tasks may
>      * decide to not do anything because the data structures are already
>      * fully optimized. We may eventually want to extend the auto
>      * condition to also cover non-auto runs so that we can detect such
>      * cases.
>      /
>
> Patrick

Sure this makes sense, will add it in.

Thanks
Karthik