Re: [PATCH V2] : kernel-chktaint : Fixed space ,cosmetic change

[Thorsten Leemhuis]

Lo! Thx for taking a look at this, but it seems a few small details need
to be improved before this can be applied. A few general issues first:

* your patch v2 is afaics on top of the first patch; when doing further
revisions please merge all your changes into one patch so everything you
want to do can be done in one commit (and people that see this mail
out-of-context can easily gasp what this is all about).

* tools/debugging/kernel-chktaint was added via the docs tree, thus I
think it's best if this change takes the same route, so please CC the
docs maintainer Jonathan Corbet <corbet@xxxxxxx>
 and linux-doc@xxxxxxxxxxxxxxx
 â I might be wrong with that line of thought, but Jonathan will know
for sure.

Am 24.03.20 um 23:59 schrieb Bhaskar Chowdhury:
> Space bwtween

Typo

> the words is fixed at the bottom of the file,sentence

Missing space after the comma.

> starting with "Documentation....."
> 
> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@xxxxxxxxx>
> ---
>  tools/debugging/kernel-chktaint | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
> index 74fd3282aa1b..1af06bc0e667 100755
> --- a/tools/debugging/kernel-chktaint
> +++ b/tools/debugging/kernel-chktaint
> @@ -198,6 +198,6 @@ fi
>  echo "Raw taint value as int/string: $taint/'$out'"

This used to be the last time. Did you move it upwards on purpose?

>  echo

If you add a blank line here you IMHO might want to add one above the
""Raw taint value" as well.

>  echo "For a more detailed explanation of the various taint flags see below pointers:"
> -echo "1) Documentation/admin-guide/tainted-kernels.rst in  the Linux kernel sources"
> +echo "1) Documentation/admin-guide/tainted-kernels.rst in the Linux kernel sources"
>  echo "2)  https://kernel.org/doc/html/latest/admin-guide/tainted-kernels.html";
There are two spaces here as well: "2)  https".

And I for one dislike the "1)" "2)" style, as in the end it's the same
file in different locations. How about a text like this instead:

```
For a more detailed explanation of the various taint flags see
Documentation/admin-guide/tainted-kernels.rst in the Linux sources which
is also available online as rendered webpage at
https://kernel.org/doc/html/latest/admin-guide/tainted-kernels.html";
```

Feel free to improve on that, it's just a suggestion. I for one wonder
if this cosmetic change is worth all of this, but no worries.

Note, you also want to change tainted-kernels.rst, as in once place it
shows the output of this tool (which is one more reason to route this
via the docs tree).

Ciao, Thorsten