On Thu, Mar 07, 2024 at 01:38:56PM +0100, Toon claes wrote:
> > diff --git a/reftable/stack.c b/reftable/stack.c
> > index 977336b7d5..40129da16c 100644
> > --- a/reftable/stack.c
> > +++ b/reftable/stack.c
> > @@ -827,51 +827,57 @@ uint64_t reftable_stack_next_update_index(struct reftable_stack *st)
> >
> > static int stack_compact_locked(struct reftable_stack *st,
> > size_t first, size_t last,
> > - struct strbuf *temp_tab,
> > - struct reftable_log_expiry_config *config)
> > + struct reftable_log_expiry_config *config,
> > + struct tempfile **temp_table_out)
> > {
> > struct strbuf next_name = STRBUF_INIT;
> > - int tab_fd = -1;
> > + struct strbuf table_path = STRBUF_INIT;
> > struct reftable_writer *wr = NULL;
> > + struct tempfile *temp_table;
> > + int temp_table_fd;
>
> Just one small nit, if you don't mind? In PATCH 2/4 you use
> `struct tempfile *tab_file` and `int tab_fd`. I would like to see
> consistency and use similar names. Personally I don't like table being
> shortened to "tab", and I think you feel the same as you've renamed the
> parameter from this function.
Yeah, I'm always a proponent of descriptive, unabbreviated names and
would thus rather want to use "temp_table" or something like that. But
in 2/4 I decided to stick with "tab_file" because there is a bunch of
already existing variables in that function which are called similar --
"temp_tab_file_name", "tab_file_name" and "tab_fd". But renaming all of
them to ensure function-level consistency would create a lot of churn.
I'll thus instead rename variables over here to their abbreviated
versions.
Patrick
Attachment:
signature.asc
Description: PGP signature
