On Tue, Aug 26, 2025 at 01:06:41PM +0530, Meet Soni wrote: > The implementation of `git pack-refs` is monolithic within > `cmd_pack_refs()`, making it impossible to share its logic with other > commands. To enable code reuse for the upcoming `git refs optimize` > subcommand, refactor the core logic into a shared helper function. > > Introduce a new `pack-refs.h` header to define the public interface > for this shared logic. It contains the declaration for a new helper > function, `pack_refs_core()`, and a macro for the common usage > options. > > Move the option parsing and packing logic from `cmd_pack_refs()` into a > new helper function named `pack_refs_core()`. This helper is made > generic by accepting the command's usage string as a parameter. > > The original `cmd_pack_refs()` is simplified to a thin wrapper that > is only responsible for defining its specific usage array and calling > the shared helper. > > Mentored-by: Patrick Steinhardt <ps@xxxxxx> > Mentored-by: shejialuo <shejialuo@xxxxxxxxx> > Signed-off-by: Meet Soni <meetsoni3017@xxxxxxxxx> > --- > builtin/pack-refs.c | 31 ++++++++++++++++++++----------- > pack-refs.h | 22 ++++++++++++++++++++++ > 2 files changed, 42 insertions(+), 11 deletions(-) > create mode 100644 pack-refs.h Shouldn't that header live in "builtin/pack-refs.h"? Makes it way more obvious that it exposes functions from "builtin/pack-refs.c". > diff --git a/pack-refs.h b/pack-refs.h > new file mode 100644 > index 0000000000..ba51d154e3 > --- /dev/null > +++ b/pack-refs.h > @@ -0,0 +1,22 @@ > +#ifndef PACK_REFS_H > +#define PACK_REFS_H > + > +struct repository; Let's add a newline here. > +/* > + * Shared usage string for options common to git-pack-refs(1) > + * and git-refs-optimize(1). The command-specific part (e.g., "git refs optimize ") > + * must be prepended by the caller. > + */ > +#define PACK_REFS_OPTS \ > + "[--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]" > + > +/* > + * The core logic for pack-refs and its clones And a dot after to terminate the sentence. > + */ > +int pack_refs_core(int argc, > + const char **argv, > + const char *prefix, > + struct repository *repo, > + const char * const *usage_opts); > + > +#endif /* PACK_REFS_H */ Patrick
