Re: [PATCH v2] trace2: don't include "fsync" events in all trace2 logs

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> As we're needing to indent the trace2_data_intmax() lines let's
> introduce helper variables to ensure that our resulting lines (which
> were already too) don't exceed the recommendations of the

too -> too something

> +    elsif ($event eq 'data_json') {
> +	# NEEDSWORK: Ignore due to
> +	# compat/win32/trace2_win32_process_info.c, which should log a
> +	# "cmd_ancestry" event instead.
> +    }

This does read better.

>  void trace_git_fsync_stats(void)
>  {
> -	trace2_data_intmax("fsync", the_repository, "fsync/writeout-only", count_fsync_writeout_only);
> -	trace2_data_intmax("fsync", the_repository, "fsync/hardware-flush", count_fsync_hardware_flush);
> +	const struct repository *r = the_repository;
> +	const intmax_t cfwo = count_fsync_writeout_only;
> +	const intmax_t cfhf = count_fsync_hardware_flush;
> +
> +	if (cfwo)
> +		trace2_data_intmax("fsync", r, "fsync/writeout-only", cfwo);
> +	if (cfhf)
> +		trace2_data_intmax("fsync", r, "fsync/hardware-flush", cfhf);

I would have wrapped the lines instead of introducing these extra
variables; the key observation is that it would make a much easier
pattern to follow for a future developer who needs to add the third
kind of "fsync" on top of the existing wo and hf.

But we can address that when somebody actually adds the third one,
so let's take the patch as-is.

Thanks.