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:

quoted

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