Re: [PATCH 40/49] builtin/repack.c: introduce `struct write_pack_opts`
- From
Jeff King <peff@peff.net>
- Date
- Oct 15, 2025, 10:28 UTC
- Message-ID
- <20251015102833.GB2250228@coredump.intra.peff.net>
- In-Reply-To
- <109109618687efeb932bee7dd5882557ab0576f4.1759097191.git.me@ttaylorr.com>
On Sun, Sep 28, 2025 at 06:09:51PM -0400, Taylor Blau wrote:
Show 5 quoted lines
> 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...
Show 7 quoted lines
> + 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.
-Peff