From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 24 Oct 2025 05:45:27 GMT
Subject: Re: [PATCH v2 9/9] builtin/maintenance: introduce "geometric" strategy
Message-ID: <aPsSd_4iFogbE0FL@pks.im>
In-Reply-To: <aPqi71+zRE2KsnCk@nand.local>

On Thu, Oct 23, 2025 at 05:49:35PM -0400, Taylor Blau wrote:
> On Tue, Oct 21, 2025 at 04:13:31PM +0200, Patrick Steinhardt wrote:
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index aaff0bae15..9739bb0ea2 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -1878,12 +1878,31 @@ static const struct maintenance_strategy incremental_strategy = {
> >  	},
> >  };
> >
> > +static const struct maintenance_strategy geometric_strategy = {
> > +	.tasks = {
> > +		[TASK_COMMIT_GRAPH].type = MAINTENANCE_TYPE_SCHEDULED | MAINTENANCE_TYPE_MANUAL,
> > +		[TASK_COMMIT_GRAPH].schedule = SCHEDULE_HOURLY,
> > +		[TASK_GEOMETRIC_REPACK].type = MAINTENANCE_TYPE_SCHEDULED | MAINTENANCE_TYPE_MANUAL,
> > +		[TASK_GEOMETRIC_REPACK].schedule = SCHEDULE_DAILY,
> > +		[TASK_PACK_REFS].type = MAINTENANCE_TYPE_SCHEDULED | MAINTENANCE_TYPE_MANUAL,
> > +		[TASK_PACK_REFS].schedule = SCHEDULE_DAILY,
> > +		[TASK_RERERE_GC].type = MAINTENANCE_TYPE_SCHEDULED | MAINTENANCE_TYPE_MANUAL,
> > +		[TASK_RERERE_GC].schedule = SCHEDULE_WEEKLY,
> > +		[TASK_REFLOG_EXPIRE].type = MAINTENANCE_TYPE_SCHEDULED | MAINTENANCE_TYPE_MANUAL,
> > +		[TASK_REFLOG_EXPIRE].schedule = SCHEDULE_WEEKLY,
> > +		[TASK_WORKTREE_PRUNE].type = MAINTENANCE_TYPE_SCHEDULED | MAINTENANCE_TYPE_MANUAL,
> > +		[TASK_WORKTREE_PRUNE].schedule = SCHEDULE_WEEKLY,
> > +	},
> > +};
> > +
> 
> What you wrote here all makes sense to me, so I don't have any comments
> on the technical content of 'geometric_strategy'.
> 
> As an aside, I wonder if we should use a nested designated initializer
> here? It seems a little cleaner to me than doing:
> 
>     .tasks = {
>         [TASK_FOO].type = ...,
>         [TASK_FOO].schedule = ...,
>     }
> 
> It's inconsistent with the style of the rest of this file, so if you did
> make this change I'd suggest adding a prerequisite change that modifies
> existing strategies to match the new style. But you could imagine
> something like the following on top:

It's more verbose, but it indeed reads a lot nicer. I'll take your
suggestion, thanks!

Patrick