On Wed, Oct 15, 2025 at 06:28:33AM -0400, Jeff King wrote:

On Sun, Sep 28, 2025 at 06:09:51PM -0400, Taylor Blau wrote:

quoted

Instead of repeating those arguments for each function, let's extract an
options struct called "write_pack_opts" which has these three parameters
as member fields. While we're at it, add fields for "packdir," and
"packtmp", both of which are static variables within the builtin, and
need to be read from within these two functions.

Makes sense, although...

quoted

+		struct write_pack_opts opts = {
+			.po_args = &po_args,
+			.destination = filter_to,
+			.pack_prefix = find_pack_prefix(packdir, packtmp),
+			.packdir = packdir,
+			.packtmp = packtmp,
+		};

...since we are now passing packdir and packtmp anyway, and pack_prefix
is derived from those, should the called function just do that
derivation itself? Or do we expect that some callers may eventually use
a different prefix?

Probably not a huge deal either way, but maybe an easy way to tighten up
the interface a bit.

We do this a little further down in "repack: move `find_pack_prefix()`
out of the builtin", which I tried to separate since I wanted to go
incrementally here. But I should mention that that patch is upcoming to
avoid readers wondering why we're not making the change sooner.

Thanks,
Taylor