After commit 45107ff6d526 ("checkpatch: if no filenames then read stdin"),
if no filenames are given, it will read patch from stdin rather than exit
directly, it is a bit confusing whether the script hangs, I do not quite
know what to do next util I understand the code logic.
At the beginning, I want to print some info if no filenames are given [1],
but as Joe Perches said, this is unnecessary. It's like trying to make cat
without command line arguments emit something.
So as Lukas Bulwahn suggested, add description for somebody that actually
reads the available kernel documentation on checkpatch.
[1] https://lore.kernel.org/patchwork/patch/1429026/
Signed-off-by: Tiezhu Yang <[email protected]>
---
Documentation/dev-tools/checkpatch.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index 51fed1b..181b95e 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -210,6 +210,8 @@ Available options:
Display the help text.
+When FILE is -, or no filenames are given, read standard input.
+
Message Levels
==============
--
2.1.0
Hey,
On Mon, May 17, 2021 at 9:30 AM Tiezhu Yang <[email protected]> wrote:
>
> After commit 45107ff6d526 ("checkpatch: if no filenames then read stdin"),
> if no filenames are given, it will read patch from stdin rather than exit
> directly, it is a bit confusing whether the script hangs, I do not quite
> know what to do next util I understand the code logic.
util -> until
>
> At the beginning, I want to print some info if no filenames are given [1],
> but as Joe Perches said, this is unnecessary. It's like trying to make cat
> without command line arguments emit something.
>
> So as Lukas Bulwahn suggested, add description for somebody that actually
> reads the available kernel documentation on checkpatch.
>
> [1] https://lore.kernel.org/patchwork/patch/1429026/
>
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
> Documentation/dev-tools/checkpatch.rst | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> index 51fed1b..181b95e 100644
> --- a/Documentation/dev-tools/checkpatch.rst
> +++ b/Documentation/dev-tools/checkpatch.rst
> @@ -210,6 +210,8 @@ Available options:
>
> Display the help text.
>
> +When FILE is -, or no filenames are given, read standard input.
> +
The addition is reasonable but the position of the text is a bit weird.
Let's have it after the Usage:: text:
-----------
diff --git a/Documentation/dev-tools/checkpatch.rst
b/Documentation/dev-tools/checkpatch.rst
index d4bb55723a86..7bf1e48207ce 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -22,6 +22,8 @@ Usage::
./scripts/checkpatch.pl [OPTION]... [FILE]...
+When FILE is -, or absent, checkpatch reads from standard input.
+
Available options:
- -q, --quiet
@@ -210,7 +212,6 @@ Available options:
Display the help text.
-When FILE is -, or no filenames are given, read standard input.
Message Levels
==============
-------------
Thanks,
Dwaipayan.
On Mon, May 17, 2021 at 8:21 AM Dwaipayan Ray <[email protected]> wrote:
>
> Hey,
>
> On Mon, May 17, 2021 at 9:30 AM Tiezhu Yang <[email protected]> wrote:
> >
> > After commit 45107ff6d526 ("checkpatch: if no filenames then read stdin"),
> > if no filenames are given, it will read patch from stdin rather than exit
> > directly, it is a bit confusing whether the script hangs, I do not quite
> > know what to do next util I understand the code logic.
>
> util -> until
s/I understand/I understood/
> >
> > At the beginning, I want to print some info if no filenames are given [1],
> > but as Joe Perches said, this is unnecessary. It's like trying to make cat
> > without command line arguments emit something.
> >
> > So as Lukas Bulwahn suggested, add description for somebody that actually
> > reads the available kernel documentation on checkpatch.
> >
> > [1] https://lore.kernel.org/patchwork/patch/1429026/
> >
Generally, I think this commit message is a bit "too much the personal
experience report" rather than focussing on the technical motivation.
I prefer the same content with less "I" and more focus on the
technically valid arguments rather than people (your experience,
Joe's, Lukas' opinion etc.).
> > Signed-off-by: Tiezhu Yang <[email protected]>
> > ---
> > Documentation/dev-tools/checkpatch.rst | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> > index 51fed1b..181b95e 100644
> > --- a/Documentation/dev-tools/checkpatch.rst
> > +++ b/Documentation/dev-tools/checkpatch.rst
> > @@ -210,6 +210,8 @@ Available options:
> >
> > Display the help text.
> >
> > +When FILE is -, or no filenames are given, read standard input.
> > +
>
> The addition is reasonable but the position of the text is a bit weird.
> Let's have it after the Usage:: text:
>
> -----------
> diff --git a/Documentation/dev-tools/checkpatch.rst
> b/Documentation/dev-tools/checkpatch.rst
> index d4bb55723a86..7bf1e48207ce 100644
> --- a/Documentation/dev-tools/checkpatch.rst
> +++ b/Documentation/dev-tools/checkpatch.rst
> @@ -22,6 +22,8 @@ Usage::
>
> ./scripts/checkpatch.pl [OPTION]... [FILE]...
>
> +When FILE is -, or absent, checkpatch reads from standard input.
> +
> Available options:
>
> - -q, --quiet
> @@ -210,7 +212,6 @@ Available options:
>
> Display the help text.
>
> -When FILE is -, or no filenames are given, read standard input.
>
> Message Levels
> ==============
> -------------
>
Fully agree with Dwaipayan here. This is the better place this
sentence should be added. Please send a patch v2.
And if you want to contribute more, please add some typical example
how to invoke checkpatch with a filename and a good example how
checkpatch could be used reading from stdin (e.g., by piping in some
suitable git log or git show output).
Lukas