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

From: Taylor Blau <[email-masked]>
Date: Wed, 15 Oct 2025 17:21:31 -0400

On Fri, Oct 10, 2025 at 02:28:57PM +0200, Patrick Steinhardt wrote:
> On Sun, Sep 28, 2025 at 06:10:08PM -0400, Taylor Blau wrote:
> > diff --git a/repack.c b/repack.c
> > index c4326a532d..7af297ae48 100644
> > --- a/repack.c
> > +++ b/repack.c
> > @@ -77,6 +77,12 @@ const char *write_pack_opts_pack_prefix(struct write_pack_opts *opts)
> >  	return pack_prefix;
> >  }
> >
> > +int write_pack_opts_is_local(struct write_pack_opts *opts)
> > +{
> > +	const char *scratch;
> > +	return skip_prefix(opts->destination, opts->packdir, &scratch);
> > +}
> > +
> >  #define DELETE_PACK 1
> >  #define RETAIN_PACK 2
> >
>
> Now that we accept booleans in our codebase, should this function return
> a `bool` instead?

Yeah, I agree that it should eventually return "bool", but I don't want
to make the change in this patch, at least within the context of this
series where I'm trying to keep the changes-per-patch as minimal as
possible.

> > diff --git a/repack.h b/repack.h
> > index 46d2312fa9..16f2de2ea9 100644
> > --- a/repack.h
> > +++ b/repack.h
> > @@ -40,6 +40,7 @@ struct write_pack_opts {
> >  };
> >
> >  const char *write_pack_opts_pack_prefix(struct write_pack_opts *opts);
> > +int write_pack_opts_is_local(struct write_pack_opts *opts);
>
> It would be good if all of these these new library functions could come
> with some documentation what they actually do.

Fully agreed. I think there is enough here that I'd rather pursue this
as a separate step, but at least this series moves us in a good
direction to document things by putting the functions in a clean header
that is more amenable to documentation than the builtin is.

Thanks,
Taylor