On Tue, Oct 03, 2023 at 05:54:19PM -0400, Taylor Blau wrote:
[snip]
> @@ -801,6 +814,38 @@ static int write_midx_included_packs(struct string_list *include,
> if (preferred)
> strvec_pushf(&cmd.args, "--preferred-pack=%s",
> pack_basename(preferred));
> + else if (names->nr) {
> + /* The largest pack was repacked, meaning that either
> + * one or two packs exist depending on whether the
> + * repository has a cruft pack or not.
Nit: this comment will grow stale soonish once your patch series lands
that introduces a maximum packfile size for cruft packs as there can be
arbitrarily many cruft packs from thereon.
> + * Select the non-cruft one as preferred to encourage
> + * pack-reuse among packs containing reachable objects
> + * over unreachable ones.
> + *
> + * (Note we could write multiple packs here if
> + * `--max-pack-size` was given, but any one of them
> + * will suffice, so pick the first one.)
> + */
Well, okay, you kind of acknowledge this here.
The rest of this patch series looks good to me and makes sense. I don't
really think that this comment here is worth a reroll.
Patrick
Attachment:
signature.asc
Description: PGP signature
