Subject: Re: [PATCH v7 03/28] refs/debug: trace into reflog expiry too

From: Junio C Hamano <[email-masked]>
Date: Tue, 20 Apr 2021 12:41:56 -0700

"Han-Wen Nienhuys via GitGitGadget" <[email-masked]> writes:

> From: Han-Wen Nienhuys <[email-masked]>
>
> Signed-off-by: Han-Wen Nienhuys <[email-masked]>
> ---
>  refs/debug.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 44 insertions(+), 3 deletions(-)

Nicely done.  

I as a reader of this patch do have to wonder, with the above very
limited log message material, how useful did "debug_reflog_expire()"
machinery used to be without any tracing.

It just reported the fact that expire method was called and what the
backend did as a whole, instead of reporting what the machinery
decided for each reflog entry to be pruned (or not pruned)?

Not a problem with this patch at all, and certainly it does not have
to be part of this series, but it feels very backwards, at least to
me, to have the method should_prune in ref backends.  As a function
to make a policy decision (e.g. "this has a timestamp older than X,
so needs to be pruned", "the author of this change I do not like, so
let's prune it ;-)"), it is more natural to have it as independent
as possible from the individual backends, no?