Subject: Re: [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file

From: Junio C Hamano <[email-masked]>
Date: Wed, 22 Jun 2022 08:12:17 -0700

Jeff Hostetler <[email-masked]> writes:

> (I just sent a response to your V2 before I saw your V3, so I'll
> my response here so that it doesn't get lost.)
>
>
> We should not be doing this.  This would dump every repo-related
> boolean value on every command.  I see that in V3 that you have
> a "trace" flag to control this.  But again, this seems wrong here.

I noticed that "flag" while merging this with another topic to
"seen" and found it strange, too.  It "allows" the caller to choose
which one gets logged per variable, but in a very hard-coded way; we
probably would have been much better off to have a table of what
gets logged and have repo_cfg_*() calls consult it.  Even without
configurability, at least it would move the hard-coded choice of
what gets logged from code to data.

But ...

> I already have a GIT_TRACE2_CONFIG_PARAMS and trace2.configparams
> that will dump "interesting" config values to the trace2 log.
> Just set one of them to a list of regex's.  Look at the comment above
> trace2_cmd_list_config() in trace2.h for details.

... that does sound like the right way to go.

> ...
> So if I'm reading this right, this will only print the value of
> "core.multipackindex".
>
> You can get this without the changes here using
>
> GIT_TRACE2_CONFIG_PARAMS="core.multipackindex,...any_other_values_of_interest..."
> before running the command (or use the config setting) before running
> your commandss.

Thanks.