Messages in this thread | | | From | Thorsten Leemhuis <> | Subject | Re: [PATCH V2] : kernel-chktaint : Fixed space ,cosmetic change | Date | Wed, 25 Mar 2020 12:23:25 +0100 |
| |
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@lwn.net> and linux-doc@vger.kernel.org – 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@gmail.com> > --- > 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
| |