Re: [PATCH 1/5] Fix some typos, grammar or wording issues in the documentation

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 3, 2023, 18:30 UTC
Message-ID: <CAPig+cR2wgAsGCTphqHsf_pbM0q_xLcMx=acVD6MDKhpEt3HNA@mail.gmail.com>
In-Reply-To: <20231003082107.3002173-1-stepnem@smrk.net>

On Tue, Oct 3, 2023 at 4:28 AM Štěpán Němec <stepnem@smrk.net> wrote:

> Fix some typos, grammar or wording issues in the documentation

SubmittingPatches suggests: s/Fix/fix

Overall, these changes are welcome improvements. I've left a few minor comments below which may or may not be actionable.

Show 10 quoted lines

> Signed-off-by: Štěpán Němec <stepnem@smrk.net>
> ---
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> @@ -393,8 +393,8 @@ mailing list{security-ml}, instead of the public mailing list.
>  Learn to use format-patch and send-email if possible.  These commands
>  are optimized for the workflow of sending patches, avoiding many ways
> -your existing e-mail client that is optimized for "multipart/*" mime
> -type e-mails to corrupt and render your patches unusable.
> +your existing e-mail client (often optimized for "multipart/*" MIME
> +type e-mails) might render your patches unusable.

A subjective alternative would have been to use commas to set off the newly-parenthesized comment. Not worth a reroll.

Show 5 quoted lines

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> @@ -96,7 +96,7 @@ foo.bar= ...`) sets `foo.bar` to the empty string which `git config
>  This is useful for cases where you want to pass transitory
> -configuration options to git, but are doing so on OS's where
> +configuration options to git, but are doing so on OSes where

Since you're touching this anyhow, it could probably be made clearer by simply spelling it out: "operating systems"

>  other processes might be able to read your cmdline
>  (e.g. `/proc/self/cmdline`), but not your environ
>  (e.g. `/proc/self/environ`). That behavior is the default on

I wonder if "cmdline" and "environ" would be better spelled out, as well, since the examples which immediately follow them give the necessary context.

    other processes might be able to read your command-line
    (e.g. `/proc/self/cmdline`), but not your environment
    (e.g. `/proc/self/environ`).

But perhaps that's outside the scope of this patch?

Show 6 quoted lines

> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> @@ -1151,7 +1151,7 @@ will be stored via placeholder `%P`.
>  This attribute controls the length of conflict markers left in
> -the work tree file during a conflicted merge.  Only setting to
> +the work tree file during a conflicted merge.  Only setting
>  the value to a positive integer has any meaningful effect.

Or, more simply:

    Only a positive integer has a meaningful effect.

Show 5 quoted lines

> diff --git a/contrib/README b/contrib/README
> @@ -24,14 +24,14 @@ lesser degree various foreign SCM interfaces, so you know the
>  I expect that things that start their life in the contrib/ area
> -to graduate out of contrib/ once they mature, either by becoming
> +graduate out of contrib/ once they mature, either by becoming

You probably want to add a comma after "area".

Do we want to correct the formatting of this pathname:

    ...in the `contrib/` area...
    ...out of `contrib/` once...

or is that outside the scope of this patch?

> diff --git a/strbuf.h b/strbuf.h
> @@ -12,9 +12,9 @@
> - * strbuf's are meant to be used with all the usual C string and memory
> + * strbufs are meant to be used with all the usual C string and memory

Alternatively:

    strbuf is meant to be used...

Show 5 quoted lines

>   * APIs. Given that the length of the buffer is known, it's often better to
> - * use the mem* functions than a str* one (memchr vs. strchr e.g.).
> + * use the mem* functions than a str* one (e.g., memchr vs. strchr).
>   * Though, one has to be careful about the fact that str* functions often
>   * stop on NULs and that strbufs may have embedded NULs.

Similar:

    ... and that a strbuf may have...

Show 7 quoted lines

> diff --git a/t/README b/t/README
> @@ -262,8 +262,8 @@ The argument for --run, <test-selector>, is a list of description
>  suite to include (or exclude, if negated) in the run.  A range is two
> -numbers separated with a dash and matches a range of tests with both
> -ends been included.  You may omit the first or the second number to
> +numbers separated with a dash and matches an inclusive range of tests
> +to run.  You may omit the first or the second number to

Not the fault of this patch, but "matches" seems an odd choice. Perhaps "specifies" would feel more natural.

>  The argument to --run is split on commas into separate strings,
> @@ -579,11 +579,10 @@ This test harness library does the following things:
> -Here are some recommented styles when writing test case.

Do you want to fix the spelling error while you're here or is that done in a later patch?

    s/recommented/recommended/

> - - Keep test title the same line with test helper function itself.
> + - Keep test titles and helper function invocations on the same line.

This would be clearer if it was switched around. Either:

    Keep the test_expect_* function call and test title on the same line.

or, more verbosely:

   Place the test title on the same line as the test_expect_* call
   which precedes it.