On Tue, 2 Sept 2025 at 15:48, Patrick Steinhardt [off-list ref] wrote:

On Tue, Aug 26, 2025 at 01:06:41PM +0530, Meet Soni wrote:

quoted

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".

I couldn't find any header files in the builtin/ directory. Also, since we
placed the for-each-ref.h file in the root directory in our previous series, I
decided to do the same here.

quoted

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.

quoted

+/*
+ * 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.

quoted

+ */
+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

Thanks
Meet