Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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.