All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Documentation: checkpatch: add description if no filenames are given
@ 2021-05-17  4:00 Tiezhu Yang
  2021-05-17  6:21 ` Dwaipayan Ray
  0 siblings, 1 reply; 3+ messages in thread
From: Tiezhu Yang @ 2021-05-17  4:00 UTC (permalink / raw)
  To: Jonathan Corbet, Joe Perches, Dwaipayan Ray, Lukas Bulwahn
  Cc: linux-kernel, linux-doc

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 <yangtiezhu@loongson.cn>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Documentation: checkpatch: add description if no filenames are given
  2021-05-17  4:00 [PATCH] Documentation: checkpatch: add description if no filenames are given Tiezhu Yang
@ 2021-05-17  6:21 ` Dwaipayan Ray
  2021-05-17  7:12   ` Lukas Bulwahn
  0 siblings, 1 reply; 3+ messages in thread
From: Dwaipayan Ray @ 2021-05-17  6:21 UTC (permalink / raw)
  To: Tiezhu Yang
  Cc: Jonathan Corbet, Joe Perches, Lukas Bulwahn, linux-kernel, linux-doc

Hey,

On Mon, May 17, 2021 at 9:30 AM Tiezhu Yang <yangtiezhu@loongson.cn> 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 <yangtiezhu@loongson.cn>
> ---
>  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.

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Documentation: checkpatch: add description if no filenames are given
  2021-05-17  6:21 ` Dwaipayan Ray
@ 2021-05-17  7:12   ` Lukas Bulwahn
  0 siblings, 0 replies; 3+ messages in thread
From: Lukas Bulwahn @ 2021-05-17  7:12 UTC (permalink / raw)
  To: Dwaipayan Ray
  Cc: Tiezhu Yang, Jonathan Corbet, Joe Perches, linux-kernel,
	open list:DOCUMENTATION

On Mon, May 17, 2021 at 8:21 AM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
>
> Hey,
>
> On Mon, May 17, 2021 at 9:30 AM Tiezhu Yang <yangtiezhu@loongson.cn> 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 <yangtiezhu@loongson.cn>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-17  7:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17  4:00 [PATCH] Documentation: checkpatch: add description if no filenames are given Tiezhu Yang
2021-05-17  6:21 ` Dwaipayan Ray
2021-05-17  7:12   ` Lukas Bulwahn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.