On 8/18/2020 8:36 PM, Junio C Hamano wrote:
> Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:
>
>>> @@ -66,6 +68,10 @@ OPTIONS
>>> --quiet::
>>> Do not report progress or other information over `stderr`.
>>>
>>> +--task=<task>::
>>> + If this option is specified one or more times, then only run the
>>> + specified tasks in the specified order.
>>
>> We should list the accepted tasks somewhere but maybe this can wait
>> until after part 2.
It's hard to see from the patch-context, but there is a section titled
"TASKS" that lists the 'gc' and 'commit-graph' tasks from the earlier
patches. Would a reference to that section be helpful?
See the TASKS section for the list of available tasks.
>>> @@ -791,7 +791,9 @@ typedef int maintenance_task_fn(struct maintenance_opts *opts);
>>> struct maintenance_task {
>>> const char *name;
>>> maintenance_task_fn *fn;
>>> - unsigned enabled:1;
>>> + unsigned enabled:1,
>>> + selected:1;
>>> + int selected_order;
>>> };
>>
>> "selected" and "selected_order" are redundant in some cases - I think
>> this would be better if selected_order is negative if this task is not
>> selected, and non-negative otherwise.
>
> It is good to get rid of redundancies.
>
>> Apart from that, maybe this should be documented. It is unusual (to me)
>> that a selection can override something being enabled, but that is the
>> case here.
>
> I had the same reaction as "(to me)" above during an earlier review
> round, IIRC. This definitely deserves documentation.
Will do. Thanks.
-Stolee
