Re: [PATCH v3] usage: add trace2 entry upon warning()

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

Jonathan Tan <jonathantanmy@google.com> writes:

> Emit a trace2 error event whenever warning() is called, just like when
> die(), error(), or usage() is called.
>
> This helps debugging issues that would trigger warnings but not errors.
> In particular, this might have helped debugging an issue I encountered
> with commit graphs at $DAYJOB [1].
>
> There is a tradeoff between including potentially relevant messages and
> cluttering up the trace output produced. I think that warning() messages
> should be included in traces, because by its nature, Git is used over
> multiple invocations of the Git tool, and a failure (currently traced)
> in a Git invocation might be caused by an unexpected interaction in a
> previous Git invocation that only has a warning (currently untraced) as
> a symptom - as is the case in [1].
>
> [1] https://lore.kernel.org/git/20200629220744.1054093-1-jonathantanmy@google.com/
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> Thanks, Junio. That comment looks good. Here is the version with Junio's
> suggested comment included, for everyone's reference.

Heh, I meant it as more of a #leftoverbit, not directly applicable to
this particular patch, but would be a good follow-up topic, as I would
have expected that die/warn/error should lose their own comments where
they call trace2_cmd_error_va() in the same patch that adds the comment
for callers near the function.

Let's use v2 if the difference between v2 and v3 is only the
addition of the comment before trace2_cmd_error_va() function decl
to help the callers.