From: Jeff King <peff@peff.net>
Date: Wed, 15 Oct 2025 10:35:16 GMT
Subject: Re: [PATCH 43/49] repack: extract `write_pack_opts_is_local()`
Message-ID: <20251015103516.GE2250228@coredump.intra.peff.net>
In-Reply-To: <1fbe3d0d7f37840b2d9faf6f62e94abd6a664ded.1759097191.git.me@ttaylorr.com>

On Sun, Sep 28, 2025 at 06:10:08PM -0400, Taylor Blau wrote:

> Similar to the previous commit, the functions `write_cruft_pack()` and
> `write_filtered_pack()` both compute a "local" variable via the exact
> same mechanism:
> 
>     const char *scratch;
>     int local = skip_prefix(opts->destination, opts->packdir, &scratch);
> 
> Not only does this cause us to repeat the same pair of lines, it also
> introduces an unnecessary "scratch" variable that is common between both
> functions.

Hmm. If we are not looking at "scratch", then does that mean
skip_prefix() is the wrong function to use? I.e., should this just be:

  int local = starts_with(opts->destinations, opts->packdir);

?

It may still be worth pulling into a separate function to encapsulate
the policy logic, but that function could be using starts_with() itself.

-Peff