Re: [PATCH 43/49] repack: extract `write_pack_opts_is_local()`

[Taylor Blau <me@ttaylorr.com>]

On Wed, Oct 15, 2025 at 06:35:16AM -0400, Jeff King wrote:
> 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.

When responding to Patrick's message on this patch I was inclined to not
make any more changes in this patch than strictly necessary. But having
two suggestions which are both easy to make, and (more importantly) easy
to verify the correctness of pushed me to go ahead and make both
changes.

Thanks,
Taylor