Re: [PATCH 1/9] tmp_objdir: add a helper function for discarding all contained objects

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 21, 2021, 23:26 UTC
Message-ID: <xmqqmtkto7yf.fsf@gitster.g>
In-Reply-To: <fab1b2c69eafbd3f211745886786c1d0ebdc05c2.1640109948.git.gitgitgadget@gmail.com>

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 21 quoted lines

> From: Elijah Newren <newren@gmail.com>
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  tmp-objdir.c | 5 +++++
>  tmp-objdir.h | 6 ++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/tmp-objdir.c b/tmp-objdir.c
> index 3d38eeab66b..adf6033549e 100644
> --- a/tmp-objdir.c
> +++ b/tmp-objdir.c
> @@ -79,6 +79,11 @@ static void remove_tmp_objdir_on_signal(int signo)
>  	raise(signo);
>  }
>  
> +void tmp_objdir_discard_objects(struct tmp_objdir *t)
> +{
> +	remove_dir_recursively(&t->path, REMOVE_DIR_KEEP_TOPLEVEL);
> +}
> +

OK.

Without a caller, it is a bit hard to judge if a separate helper makes the caller easier to read and understand, or becomes an extra layer of abstraction that obscures the logic. Hopefully, having a more specific function name with "tmp" and "discard" in it makes the intent at callers more clear than the function that is named after the detail of the operation.

Show 9 quoted lines

> +/*
> + * Remove all objects from the temporary object directory, while leaving it
> + * around so more objects can be added.
> + */
> +void tmp_objdir_discard_objects(struct tmp_objdir *);
> +
>  /*
>   * Add the temporary object directory as an alternate object store in the
>   * current process.