Subject: Re: [PATCH 03/16] refs: implement releasing ref storages

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 17 May 2024 09:09:21 +0200

On Thu, May 16, 2024 at 11:01:01AM -0700, Junio C Hamano wrote:
> Karthik Nayak <[email-masked]> writes:
> 
> >> +static void debug_release(struct ref_store *refs)
> >> +{
> >> +	struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
> >
> > We should probably add a trace here, using `trace_printf_key()`
> 
> A totally ignorant question.  Should we be adding more traces with
> trace_* API instead of trace2_* API?  If the latter aims to cover
> superset of use cases the former did, I was hoping that we can
> eventually deprecate the former, hence this question.  Of course We
> could add a compatiblity layer that emulates trace_* API with a thin
> wrapper around trace2_* API, but if we do not add new callers, it
> may still be feasible to directly migrate the callers to use trace2_
> API without having to invent such compatibility wrappers.

I cannot really answer this question as I ain't got much of a clue
around the tracing APIs. But in this case I think we should indeed add
this via `trace_printf_key()` so that we remain consistent with all the
other wrappers in the debug store. I'd argue that either all functions
here should use trace or trace2, but not a mixture.

Patrick