Re: [PATCH 01/14] trace2: Documentation/technical/api-trace2.txt

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jan 25, 2019, 13:19 UTC
Message-ID: <20190125131929.GC6702@szeder.dev>
In-Reply-To: <1a90de9dab0dd836e54fee9e08ab9e2284e1027a.1548192131.git.gitgitgadget@gmail.com>

On Tue, Jan 22, 2019 at 01:22:12PM -0800, Jeff Hostetler via GitGitGadget wrote:

Show 28 quoted lines

> From: Jeff Hostetler <jeffhost@microsoft.com>
> 
> Created design document for Trace2 feature.
> 
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
>  Documentation/technical/api-trace2.txt | 1158 ++++++++++++++++++++++++
>  1 file changed, 1158 insertions(+)
>  create mode 100644 Documentation/technical/api-trace2.txt
> 
> diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
> new file mode 100644
> index 0000000000..501fd770f2
> --- /dev/null
> +++ b/Documentation/technical/api-trace2.txt
> @@ -0,0 +1,1158 @@
> +Trace2 API
> +==========
> +
> +The Trace2 API can be used to print debug, performance, and telemetry
> +information to stderr or a file.  The Trace2 feature is inactive unless
> +explicitly enabled by setting one or more of the `GIT_TR2`, `GIT_TR2_PERF`,
> +or `GIT_TR2_EVENT` environment variables.
> +
> +The Trace2 API is intended to replace the existing (Trace1)
> +printf-style tracing provided by the existing `GIT_TRACE` and
> +`GIT_TRACE_PERFORMANCE` facilities.  During initial implementation,
> +Trace2 and Trace1 may operate in parallel.

Speaking of replacing Trace1, I couldn't find (or managed to overlook) the Trace2 equivalent of the good old "plain"

  trace_printf("Uh-oh!");

which is my go-to tool when chasing elusive heisenbugs and attempting to understand racy situations and flaky tests.

Show 7 quoted lines

> +Git Command Detail Events::
> +
> +	These are concerned with describing the specific Git command
> +	after the command line, config, and environment are inspected.
> ++
> +----------------
> +trace2_cmd_verb(...)

What is a "verb"?

If it means "command", then just call it so. Please stick to established Git terminology instead of introducing unnecessary new terms.

> +trace2_cmd_subverb(...)

What is a "subverb"?

Looking at the strings passed to this function in later patches, I am only sure in one thing: that it is not, in fact, a verb :)

I think that in general we are better off without the word "verb", e.g.:

> +	After the basics are established, additional process
> +	information can be sent to Trace2 as it is discovered, such as
> +	the command verb, alias expansion, interesting config

The word 'verb' doesn't add any value to the sentence, but s/ verb// does make it clearer.

> +When a git process is a (direct or indirect) child of another
> +git process, it inherits Trace2 context information.  This
> +allows the child to print a verb hierarchy.

Without s/verb/command/ this sentence doesn't make much sense to begin with.