Re: [PATCH v2 2/2] wrapper: use trace2 counters to collect fsync stats

On 7/25/23 3:31 PM, Junio C Hamano wrote:

Junio C Hamano [off-list ref] writes:

quoted

I also spotted this change since v1:

- Rename trace2 counters to use "-" (not "_") as inter-word separators.

Since I do not seem to be able to find any review comments regarding
the variable naming in the v1's thread, let's ask stakeholders.

Are folks involved in the trace2 subsystem (especially Jeff
Hostetler---already CC:ed---who presumably has the most stake in it)
OK with the naming convention of the multi-word variable?  This is
the first use of multi-word variable name in tr2_ctr, and thus will
establish whatever convention you guys want to use.  I do have a
slight preference of "writeout-only" over "writeout_only" but that
is purely from visual appearance.  If there is a desire to keep the
names literally reusable as identifiers in some languages used to
postprocess trace output, or something, that might weigh
differently.

I heard absolutely nothing since I asked the above question last
week, so I'll take the absense of response as absense of interest in
the way how names are spelled.

Therefore, let me make a unilateral declaration here ;-)  The trace2
counters with multi-word names are to be named using "-" as their
inter-word separators.  Any patch that adds new counters that do not
follow the convention will silently dropped on the floor from now on.

Let's move this patch forward by merging to 'next' soonish.

Thanks.

Sorry I missed before I left for vacation.

Multi-word terms have unfortunately used both "-" and "_"
separators in the past (e.g. builtin/pack-objects.c)
I don't think it really matters one way or the other.

Originally, I used "_" because there were places where the
post-processing could more easily extract or query a nested JSON
or Kusto expression without needing escapes. For example
`<record>.<category>.<item>` rather than something like
`<record>["<category>"]["<item>"]` to avoid having the dash
interpreted as subtraction on a local variable).

But as I and others have added other categories and messages,
we've drifted from that usage.  And that is fine.

Thanks
Jeff