Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A couple of fixes for the Trace2 documentation #1172

Closed
wants to merge 6 commits into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Mar 10, 2022

While verifying a suggestion for a contributor before sending it, I noticed that the Trace2 documentation contained an incorrect example. Looking around, I found a couple other things I wanted to fix, so here are the patches.

Note: This patch series is not intended to be perfectionist. I just want the result to be good enough, and move on to more important things.

Changes since v1:

  • Added an Oxford comma (at least I think it is a stylistic one, not a required one, but then, all three people involved in the discussion are non-native speakers, so there).
  • Added a patch to use a consistent style for function names.
  • I now surround even more terms in backticks.
  • I spotted and fixed a missing full stop, too.

Cc: Jeff Hostetler jeffhost@microsoft.com
cc: Jeff Hostetler git@jeffhostetler.com
cc: Ævar Arnfjörð Bjarmason avarab@gmail.com

@gitgitgadget

This comment was marked as resolved.

@gitgitgadget

This comment was marked as resolved.

@gitgitgadget

This comment was marked as resolved.

@gitgitgadget

This comment was marked as resolved.

@dscho
Copy link
Member Author

dscho commented Mar 10, 2022

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 10, 2022

Submitted as pull.1172.git.1646919331.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1172/dscho/fix-trace2-docs-v1

To fetch this version to local tag pr-1172/dscho/fix-trace2-docs-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1172/dscho/fix-trace2-docs-v1

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 10, 2022

On the Git mailing list, Jeff Hostetler wrote (reply to this):



On 3/10/22 8:35 AM, Johannes Schindelin via GitGitGadget wrote:
> While verifying a suggestion for a contributor before sending it, I noticed
> that the Trace2 documentation contained an incorrect example. Looking
> around, I found a couple other things I wanted to fix, so here are the
> patches.
> > Johannes Schindelin (4):
>    trace2 docs: a couple of grammar fixes
>    trace2 docs: surround more terms in backticks
>    trace2 docs: fix a JSON formatted example
>    trace2 docs: clarify what `varargs` is all about
> >   Documentation/technical/api-trace2.txt | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
> > > base-commit: 1a4874565fa3b6668042216189551b98b4dc0b1b
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1172%2Fdscho%2Ffix-trace2-docs-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1172/dscho/fix-trace2-docs-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1172
> LGTM

Thanks
Jeff

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 10, 2022

User Jeff Hostetler <git@jeffhostetler.com> has been added to the cc: list.

@@ -24,7 +24,7 @@ for example.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Thu, Mar 10 2022, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Documentation/technical/api-trace2.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
> index 285f713ed84..91985512867 100644
> --- a/Documentation/technical/api-trace2.txt
> +++ b/Documentation/technical/api-trace2.txt
> @@ -170,7 +170,7 @@ Some functions have a `_va_fl()` suffix to indicate that they also
>  take a `va_list` argument.
>  
>  Some functions have a `_printf_fl()` suffix to indicate that they also
> -take a varargs argument.
> +take a `printf()` style format with a variable number of arguments.

This is correct, but I think better would be to explicitly describe the
_printf_va_fl() suffix in this list, i.e. now that implicitly falls
under _va_fl(), so it's *technically* redundant.

But either describing it explicitly, or saying that `_printf_*_fl()` are
function that take a printf format before either a "va_args" or "..."
would make the end-state a bit more obvious.

I wonder if the pre-image wanted to describe `_printf_va_fl()` (which
would make it correct), and we just omitted a `printf_fl()` description.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 10, 2022

User Ævar Arnfjörð Bjarmason <avarab@gmail.com> has been added to the cc: list.

@@ -24,7 +24,7 @@ for example.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Thu, Mar 10 2022, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Documentation/technical/api-trace2.txt | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
> index bb13ca3db8b..77216eff622 100644
> --- a/Documentation/technical/api-trace2.txt
> +++ b/Documentation/technical/api-trace2.txt
> @@ -24,7 +24,7 @@ for example.
>  
>  Trace2 is controlled using `trace2.*` config values in the system and
>  global config files and `GIT_TRACE2*` environment variables.  Trace2 does
> -not read from repo local or worktree config files or respect `-c`
> +not read from repo local or worktree config files nor does it respect `-c`

Perhaps adding a comma before "nor" would also be a good addition.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

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

> On Thu, Mar 10 2022, Johannes Schindelin via GitGitGadget wrote:
>
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>>  Documentation/technical/api-trace2.txt | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
>> index bb13ca3db8b..77216eff622 100644
>> --- a/Documentation/technical/api-trace2.txt
>> +++ b/Documentation/technical/api-trace2.txt
>> @@ -24,7 +24,7 @@ for example.
>>  
>>  Trace2 is controlled using `trace2.*` config values in the system and
>>  global config files and `GIT_TRACE2*` environment variables.  Trace2 does
>> -not read from repo local or worktree config files or respect `-c`
>> +not read from repo local or worktree config files nor does it respect `-c`
>
> Perhaps adding a comma before "nor" would also be a good addition.

Yup, that sounds sensible.  All other hunks looked good, too.

Thanks, both.

@@ -24,7 +24,7 @@ for example.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Documentation/technical/api-trace2.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
> index 77216eff622..8ef921a4e4d 100644
> --- a/Documentation/technical/api-trace2.txt
> +++ b/Documentation/technical/api-trace2.txt
> @@ -70,7 +70,7 @@ $ cat ~/log.normal
>  
>  The performance format target (PERF) is a column-based format to
>  replace GIT_TRACE_PERFORMANCE and is suitable for development and

`GIT_TRACE_PERFORMANCE` to match `GIT_TRACE2_PERF` we see below.

> -testing, possibly to complement tools like gprof.  This format is
> +testing, possibly to complement tools like `gprof`.  This format is
>  enabled with the `GIT_TRACE2_PERF` environment variable or the
>  `trace2.perfTarget` system or global config setting.
>  
> @@ -172,7 +172,7 @@ take a `va_list` argument.
>  Some functions have a `_printf_fl()` suffix to indicate that they also
>  take a varargs argument.
>  
> -There are CPP wrapper macros and ifdefs to hide most of these details.
> +There are CPP wrapper macros and `#ifdef`s to hide most of these details.
>  See `trace2.h` for more details.  The following discussion will only
>  describe the simplified forms.

@@ -24,7 +24,7 @@ for example.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> @@ -34,8 +34,8 @@ Format details are given in a later section.
>  
>  === The Normal Format Target
>  
> -The normal format target is a tradition printf format and similar
> -to GIT_TRACE format.  This format is enabled with the `GIT_TRACE2`
> +The normal format target is a traditional printf format and similar
> +to the GIT_TRACE format.  This format is enabled with the `GIT_TRACE2`

In the same spirit as [2/4] and match the part that [4/4] touches,
this probably is better:

    The normal format target is a traditional `printf()` format and similar
    to the `GIT_TRACE` format.  This format is enabled with the `GIT_TRACE2`

dscho added 6 commits May 3, 2022 10:08
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We append an ellipsis and enclose it in backticks to indicate that it is
a function elsewhere, let's also use that here.

While at it, ensure the same for `waitpid()`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The example was not in valid JSON format due to a duplicate key "sid".

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member Author

dscho commented May 3, 2022

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented May 3, 2022

Submitted as pull.1172.v2.git.1651573607.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1172/dscho/fix-trace2-docs-v2

To fetch this version to local tag pr-1172/dscho/fix-trace2-docs-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1172/dscho/fix-trace2-docs-v2

@gitgitgadget
Copy link

gitgitgadget bot commented May 3, 2022

On the Git mailing list, Jeff Hostetler wrote (reply to this):



On 5/3/22 6:26 AM, Johannes Schindelin via GitGitGadget wrote:
> While verifying a suggestion for a contributor before sending it, I noticed
> that the Trace2 documentation contained an incorrect example. Looking
> around, I found a couple other things I wanted to fix, so here are the
> patches.

Nice. Thanks for your attention to detail here.

Jeff

@gitgitgadget
Copy link

gitgitgadget bot commented May 4, 2022

This branch is now known as js/trace2-doc-fixes.

@gitgitgadget
Copy link

gitgitgadget bot commented May 4, 2022

This patch series was integrated into seen via git@e81c54a.

@gitgitgadget gitgitgadget bot added the seen label May 4, 2022
@gitgitgadget
Copy link

gitgitgadget bot commented May 5, 2022

There was a status update in the "New Topics" section about the branch js/trace2-doc-fixes on the Git mailing list:

source: <pull.1172.v2.git.1651573607.gitgitgadget@gmail.com>

@gitgitgadget
Copy link

gitgitgadget bot commented May 6, 2022

This patch series was integrated into seen via git@f89ea98.

@gitgitgadget
Copy link

gitgitgadget bot commented May 10, 2022

This patch series was integrated into seen via git@6d88c24.

@gitgitgadget
Copy link

gitgitgadget bot commented May 11, 2022

This patch series was integrated into seen via git@82547d2.

@gitgitgadget
Copy link

gitgitgadget bot commented May 11, 2022

There was a status update in the "Cooking" section about the branch js/trace2-doc-fixes on the Git mailing list:

Trace2 documentation updates.

Will merge to 'next'.
source: <pull.1172.v2.git.1651573607.gitgitgadget@gmail.com>

@gitgitgadget
Copy link

gitgitgadget bot commented May 11, 2022

This patch series was integrated into seen via git@b2cec84.

@gitgitgadget
Copy link

gitgitgadget bot commented May 11, 2022

This patch series was integrated into seen via git@9a08352.

@gitgitgadget
Copy link

gitgitgadget bot commented May 12, 2022

This patch series was integrated into seen via git@e7ee378.

@gitgitgadget
Copy link

gitgitgadget bot commented May 12, 2022

This patch series was integrated into next via git@b37c6a8.

@gitgitgadget gitgitgadget bot added the next label May 12, 2022
@gitgitgadget
Copy link

gitgitgadget bot commented May 13, 2022

There was a status update in the "Cooking" section about the branch js/trace2-doc-fixes on the Git mailing list:

Trace2 documentation updates.

Will merge to 'master'.
source: <pull.1172.v2.git.1651573607.gitgitgadget@gmail.com>

@gitgitgadget
Copy link

gitgitgadget bot commented May 13, 2022

This patch series was integrated into seen via git@18d6068.

@gitgitgadget
Copy link

gitgitgadget bot commented May 16, 2022

This patch series was integrated into seen via git@d7f713e.

@gitgitgadget
Copy link

gitgitgadget bot commented May 17, 2022

There was a status update in the "Cooking" section about the branch js/trace2-doc-fixes on the Git mailing list:

Trace2 documentation updates.

Will merge to 'master'.
source: <pull.1172.v2.git.1651573607.gitgitgadget@gmail.com>

@gitgitgadget
Copy link

gitgitgadget bot commented May 18, 2022

This patch series was integrated into seen via git@102d719.

@gitgitgadget
Copy link

gitgitgadget bot commented May 18, 2022

This patch series was integrated into seen via git@e9b5669.

@gitgitgadget
Copy link

gitgitgadget bot commented May 19, 2022

This patch series was integrated into seen via git@a9ce611.

@gitgitgadget
Copy link

gitgitgadget bot commented May 20, 2022

This patch series was integrated into seen via git@722948d.

@gitgitgadget
Copy link

gitgitgadget bot commented May 21, 2022

This patch series was integrated into seen via git@3ab7328.

@gitgitgadget
Copy link

gitgitgadget bot commented May 21, 2022

This patch series was integrated into master via git@3ab7328.

@gitgitgadget
Copy link

gitgitgadget bot commented May 21, 2022

This patch series was integrated into next via git@3ab7328.

@gitgitgadget gitgitgadget bot added the master label May 21, 2022
@gitgitgadget gitgitgadget bot closed this May 21, 2022
@gitgitgadget
Copy link

gitgitgadget bot commented May 21, 2022

Closed via 3ab7328.

@dscho dscho deleted the fix-trace2-docs branch May 21, 2022 06:38
@dscho dscho self-assigned this May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant