From: Karthik Nayak Date: Thu, 16 Oct 2025 12:15:08 GMT Subject: Re: [PATCH 3/4] refs: rename 'pack_refs_opts' to 'refs_optimize_opts' Message-ID: In-Reply-To: Justin Tobler writes: > On 25/10/15 11:25PM, Karthik Nayak wrote: >> The previous commit removed all references to 'pack_refs()' within >> the refs subsystem. Continue this cleanup by also renaming >> 'pack_refs_opts' to 'refs_optimize_opts' and the respective flags >> accordingly. Keeping the naming consistent will make the code easier to >> maintain. >> >> Signed-off-by: Karthik Nayak >> --- >> pack-refs.c | 8 ++++---- >> refs.c | 2 +- >> refs.h | 16 ++++++++-------- >> refs/debug.c | 2 +- >> refs/files-backend.c | 10 +++++----- >> refs/packed-backend.c | 2 +- >> refs/refs-internal.h | 2 +- >> refs/reftable-backend.c | 4 ++-- >> 8 files changed, 23 insertions(+), 23 deletions(-) >> >> diff --git a/pack-refs.c b/pack-refs.c >> index 1a5e07d8b8..d0ffed93c1 100644 >> --- a/pack-refs.c >> +++ b/pack-refs.c >> @@ -14,10 +14,10 @@ int pack_refs_core(int argc, >> { >> struct ref_exclusions excludes = REF_EXCLUSIONS_INIT; >> struct string_list included_refs = STRING_LIST_INIT_NODUP; >> - struct pack_refs_opts pack_refs_opts = { >> + struct refs_optimize_opts pack_refs_opts = { > > We could rename the variable name here to, but probably not a big deal > either way. > Since I'm re-rolling, let me do that :) >> .exclusions = &excludes, >> .includes = &included_refs, >> - .flags = PACK_REFS_PRUNE, >> + .flags = REFS_OPTIMIZE_PRUNE, >> }; >> struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP; >> struct string_list_item *item; >> @@ -26,8 +26,8 @@ int pack_refs_core(int argc, >> >> struct option opts[] = { >> OPT_BOOL(0, "all", &pack_all, N_("pack everything")), >> - OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE), >> - OPT_BIT(0, "auto", &pack_refs_opts.flags, N_("auto-pack refs as needed"), PACK_REFS_AUTO), >> + OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), REFS_OPTIMIZE_PRUNE), >> + OPT_BIT(0, "auto", &pack_refs_opts.flags, N_("auto-pack refs as needed"), REFS_OPTIMIZE_AUTO), >> OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"), >> N_("references to include")), >> OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"), >> diff --git a/refs.c b/refs.c >> index b9a4a60646..0d0831f29b 100644 >> --- a/refs.c >> +++ b/refs.c >> @@ -2313,7 +2313,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo, >> refs->gitdir = xstrdup(path); >> } >> >> -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) >> +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts) >> { >> return refs->be->optimize(refs, opts); >> } >> diff --git a/refs.h b/refs.h >> index 04e917fec0..d2630af97f 100644 >> --- a/refs.h >> +++ b/refs.h >> @@ -500,15 +500,15 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, >> >> /* >> * Flags for controlling behaviour of refs_optimize() >> - * PACK_REFS_PRUNE: Prune loose refs after packing >> - * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end >> - * result are decided by the ref backend. Backends may ignore >> - * this flag and fall back to a normal repack. >> + * REFS_OPTIMIZE_PRUNE: Prune loose refs after packing >> + * REFS_OPTIMIZE_AUTO: Pack refs on a best effort basis. The heuristics and end >> + * result are decided by the ref backend. Backends may ignore >> + * this flag and fall back to a normal repack. >> */ >> -#define PACK_REFS_PRUNE (1 << 0) >> -#define PACK_REFS_AUTO (1 << 1) >> +#define REFS_OPTIMIZE_PRUNE (1 << 0) >> +#define REFS_OPTIMIZE_AUTO (1 << 1) >> >> -struct pack_refs_opts { >> +struct refs_optimize_opts { >> unsigned int flags; >> struct ref_exclusions *exclusions; >> struct string_list *includes; >> @@ -518,7 +518,7 @@ struct pack_refs_opts { >> * Optimize the ref store. The exact behavior is up to the backend. >> * For the files backend, this is equivalent to packing refs. >> */ >> -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts); >> +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts); > > I noticed when poking around the code that the optimize callback was > still using the `pack_refs_opts`. Nice to see this clean up. :) > >> /* >> * Setup reflog before using. Fill in err and return -1 on failure. > [snip] >> diff --git a/refs/packed-backend.c b/refs/packed-backend.c >> index 20cf9fab18..0aa0ff6701 100644 >> --- a/refs/packed-backend.c >> +++ b/refs/packed-backend.c >> @@ -1774,7 +1774,7 @@ static int packed_transaction_finish(struct ref_store *ref_store, >> } >> >> static int packed_optimize(struct ref_store *ref_store UNUSED, >> - struct pack_refs_opts *pack_opts UNUSED) >> + struct refs_optimize_opts *pack_opts UNUSED) > > Also not a big deal, but we could rename `pack_opts` here to just > `opts`. It's not even used anyways. > > All the other trivial renames in this patch look good. > > -Justin > Will do, Thanks for the review.