Re: [PATCH 43/49] repack: extract `write_pack_opts_is_local()`
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Oct 15, 2025, 21:21 UTC
- Message-ID
- <aPAQWwfw+GjdZ4Yq@nand.local>
- In-Reply-To
- <aOj8CUgapqZEhnlK@pks.im>
On Fri, Oct 10, 2025 at 02:28:57PM +0200, Patrick Steinhardt wrote:
Show 21 quoted lines
> 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.
Show 12 quoted lines
> > 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