Re: [PATCH 19/49] repack: remove 'existing_packs' API from the builtin
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Oct 10, 2025, 22:57 UTC
- Message-ID
- <aOmPROWM95FX57wC@nand.local>
- In-Reply-To
- <20251010060836.GG1965904@coredump.intra.peff.net>
On Fri, Oct 10, 2025 at 02:08:36AM -0400, Jeff King wrote:
Show 34 quoted lines
> On Sun, Sep 28, 2025 at 06:08:23PM -0400, Taylor Blau wrote:
>
> > --- a/repack.h
> > +++ b/repack.h
> > @@ -2,6 +2,7 @@
> > #define REPACK_H
> >
> > #include "list-objects-filter-options.h"
> > +#include "string-list.h"
> >
> > struct pack_objects_args {
> > char *window;
> > @@ -31,4 +32,38 @@ void pack_objects_args_release(struct pack_objects_args *args);
> > void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,
> > const char *base_name);
> >
> > +struct repository;
> > +struct packed_git;
> > +
> > +struct existing_packs {
> > + struct repository *repo;
> > + struct string_list kept_packs;
> > + struct string_list non_kept_packs;
> > + struct string_list cruft_packs;
> > +};
>
> Earlier, I saw remove_redundant_pack() got namespaced with a "repack_"
> prefix. Which I think makes sense, since it's a pretty generic name and
> the symbol now has global visibility. But none of this existing_packs
> stuff got the same treatment.
>
> I don't know how much it matters. I certainly have no plans to call
> something existing_packs elsewhere in the code, but I do wonder if the
> naming should make it more clear this is all repack-related.I think this is a similar case as `struct pack_objects_args`. We could prefix this as `repack_existing_packs`, but it seems a bit verbose, and existing_packs is a semi-useful API by itself.
Thanks, Taylor