Subject: Re: [PATCH 4/4] reftable/stack: register compacted tables as tempfiles

From: Toon claes <[email-masked]>
Date: Thu, 07 Mar 2024 13:38:56 +0100

> 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.

-- 
Toon