Re: [PATCH 38/49] builtin/repack.c: inline packs within `write_midx_included_packs()`
From: Jeff King <hidden>
Date: 2025-10-15 10:33:21
On Sun, Sep 28, 2025 at 06:09:40PM -0400, Taylor Blau wrote:
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
responsible for itself computing the list of included packs. Inline the
declaration and initialization of `included_packs` into the
`write_midx_included_packs()` function itself, and repeat that pattern
in the future when we introduce new ways to write MIDXs.OK. I thought at first this was doing something complicated, but it really is just: move the call to midx_included_packs() down into the function which uses it, rather than doing it in the caller. Without another caller we can't judge this, but I'll take your word that it helps eventually. The patch itself looks like it should be a noop with respect to behavior. -Peff