Re: [GSoC][PATCH 1/5] builtin/pack-refs: factor out core logic into a helper

>
> 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@pks.im>
> > Mentored-by: shejialuo <shejialuo@gmail.com>
> > Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
> > ---
> >  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