Re: [GSoC][PATCH 1/5] builtin/pack-refs: factor out core logic into a helper
From: Patrick Steinhardt <hidden>
Date: 2025-09-02 10:18:09
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 [off-list ref] Mentored-by: shejialuo [off-list ref] Signed-off-by: Meet Soni <redacted> --- 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".
quoted hunk ↗ jump to hunk
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