From: Jeff King <peff@peff.net>
Date: Thu, 07 Sep 2023 07:54:00 GMT
Subject: Re: [PATCH 1/8] builtin/repack.c: extract structure to store existing packs
Message-ID: <20230907075400.GA1260718@coredump.intra.peff.net>
In-Reply-To: <5b48b7e3cc03c83465a3dcecaa98b9d2e9667084.1693946195.git.me@ttaylorr.com>

On Tue, Sep 05, 2023 at 04:36:40PM -0400, Taylor Blau wrote:

> The repack machinery needs to keep track of which packfiles were present
> in the repository at the beginning of a repack, segmented by whether or
> not each pack is marked as kept.
> 
> The names of these packs are stored in two `string_list`s, corresponding
> to kept- and non-kept packs, respectively. As a consequence, many
> functions within the repack code need to take both `string_list`s as
> arguments, leading to code like this:
> 
>     ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix,
>                            cruft_expiration, &names,
>                            &existing_nonkept_packs, /* <- */
>                            &existing_kept_packs);   /* <- */
> 
> Wrap up this pair of `string_list`s into a single structure that stores
> both. This saves us from having to pass both string lists separately,
> and prepares for adding additional fields to this structure.

Makes sense. Even without any additional fields, the grouping makes the
code a bit easier to follow.

Patch is noisy, but looks correct. :)

-Peff