From: Taylor Blau <me@ttaylorr.com>
Date: Fri, 10 Oct 2025 23:02:46 GMT
Subject: Re: [PATCH 38/49] builtin/repack.c: inline packs within `write_midx_included_packs()`
Message-ID: <aOmQlkDQOYFDnZdH@nand.local>
In-Reply-To: <aOj72bpVTCXuFSHN@pks.im>

On Fri, Oct 10, 2025 at 02:28:09PM +0200, Patrick Steinhardt wrote:
> On Sun, Sep 28, 2025 at 06:09:40PM -0400, Taylor Blau wrote:
> > To write a MIDX at the end of a repack operation, 'git repack' presently
> > computes the set of packs to write into the MIDX, before invoking
> > `write_midx_included_packs()` with a `string_list` containing those
> > packs.
> >
> > The logic for computing which packs are supposed to appear in the
> > resulting MIDX is within `midx_included_packs()`, where it is aware of
> > details like which cruft pack(s) were written/combined, if/how we did a
> > geometric repack, etc.
> >
> > Computing this list ourselves before providing it to the sole function
> > to make use of that list `write_midx_included_packs()` is somewhat
> > awkward. In the future, repack will learn how to write incremental
> > MIDXs, which will use a very different pack selection routine.
> >
> > Instead of doing something like:
> >
> >     struct string_list included_packs = STRING_LIST_INIT_DUP;
> >     if (incremental) {
> >         midx_incremental_included_packs(&included_packs, ...):
> >         write_midx_incremental_included_packs(&included_packs, ...);
> >     } else {
> >         midx_included_packs(&included_packs, ...):
> >         write_midx_included_packs(&included_packs, ...);
> >     }
> >
> > in the future, let's have each function which writes a MIDX be
>
> s/writes/that &/

Good catch, "that writes" is definitely grammatically correct, but I
think the substitution is s/which/that/, not s/writes/that &/.

Thanks,
Taylor