* [PATCH] checkpatch.pl: Improve WARNING on Kconfig help @ 2018-12-19 8:35 Igor Stoppa 2018-12-19 10:44 ` Joe Perches 0 siblings, 1 reply; 9+ messages in thread From: Igor Stoppa @ 2018-12-19 8:35 UTC (permalink / raw) To: Andy Whitcroft, Joe Perches; +Cc: igor.stoppa, linux-kernel The checkpatch.pl script complains when the help section of a Kconfig entry is too short, but it doesn't really explain what it is looking for. Instead, it gives a generic warning that one should consider writing a paragraph. But what it *really* checks is that the help section is at least .$min_conf_desc_length lines long. Since the definition of what is a paragraph is not really carved in stone (and actually the primary descriptions is "5 sentences"), make the warning less ambiguous by expliciting the actual test condition, so that one doesn't have to read checkpatch.pl sources, to figure out the actual test. Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com> CC: Andy Whitcroft <apw@canonical.com> CC: Joe Perches <joe@perches.com> CC: linux-kernel@vger.kernel.org --- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c883ec55654f..e255f0423cca 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2931,7 +2931,8 @@ sub process { } if ($is_start && $is_end && $length < $min_conf_desc_length) { WARN("CONFIG_DESCRIPTION", - "please write a paragraph that describes the config symbol fully\n" . $herecurr); + "please write a paragraph (" .$min_conf_desc_length . " lines)" . + " that describes the config symbol fully\n" . $herecurr); } #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } -- 2.19.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] checkpatch.pl: Improve WARNING on Kconfig help 2018-12-19 8:35 [PATCH] checkpatch.pl: Improve WARNING on Kconfig help Igor Stoppa @ 2018-12-19 10:44 ` Joe Perches 2018-12-19 11:59 ` Andy Whitcroft 0 siblings, 1 reply; 9+ messages in thread From: Joe Perches @ 2018-12-19 10:44 UTC (permalink / raw) To: Igor Stoppa, Andy Whitcroft; +Cc: igor.stoppa, linux-kernel On Wed, 2018-12-19 at 10:35 +0200, Igor Stoppa wrote: > The checkpatch.pl script complains when the help section of a Kconfig > entry is too short, but it doesn't really explain what it is looking > for. Instead, it gives a generic warning that one should consider writing > a paragraph. > > But what it *really* checks is that the help section is at least > .$min_conf_desc_length lines long. > > Since the definition of what is a paragraph is not really carved in > stone (and actually the primary descriptions is "5 sentences"), make the > warning less ambiguous by expliciting the actual test condition, so that > one doesn't have to read checkpatch.pl sources, to figure out the actual > test. [] > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -2931,7 +2931,8 @@ sub process { > } > if ($is_start && $is_end && $length < $min_conf_desc_length) { > WARN("CONFIG_DESCRIPTION", > - "please write a paragraph that describes the config symbol fully\n" . $herecurr); > + "please write a paragraph (" .$min_conf_desc_length . " lines)" . could say "(at least $min_conf_desc_length lines)" ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] checkpatch.pl: Improve WARNING on Kconfig help 2018-12-19 10:44 ` Joe Perches @ 2018-12-19 11:59 ` Andy Whitcroft 2018-12-19 12:29 ` Joe Perches 0 siblings, 1 reply; 9+ messages in thread From: Andy Whitcroft @ 2018-12-19 11:59 UTC (permalink / raw) To: Joe Perches; +Cc: Igor Stoppa, igor.stoppa, linux-kernel On Wed, Dec 19, 2018 at 02:44:36AM -0800, Joe Perches wrote: > On Wed, 2018-12-19 at 10:35 +0200, Igor Stoppa wrote: > > The checkpatch.pl script complains when the help section of a Kconfig > > entry is too short, but it doesn't really explain what it is looking > > for. Instead, it gives a generic warning that one should consider writing > > a paragraph. > > > > But what it *really* checks is that the help section is at least > > .$min_conf_desc_length lines long. > > > > Since the definition of what is a paragraph is not really carved in > > stone (and actually the primary descriptions is "5 sentences"), make the > > warning less ambiguous by expliciting the actual test condition, so that > > one doesn't have to read checkpatch.pl sources, to figure out the actual > > test. > [] > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > @@ -2931,7 +2931,8 @@ sub process { > > } > > if ($is_start && $is_end && $length < $min_conf_desc_length) { > > WARN("CONFIG_DESCRIPTION", > > - "please write a paragraph that describes the config symbol fully\n" . $herecurr); > > + "please write a paragraph (" .$min_conf_desc_length . " lines)" . > > could say "(at least $min_conf_desc_length lines)" The original is better description in the semantic sense. We want them to describe it well. We assume they haven't because it is short. We don't want them to make it long, we want them to confirm it is fully described. You arn't trying to make people make these warnings away, they should just be checking they have met the criteria in the warning. If they have they can ignore the warning and be happy, they don't have to add two more lines. To cover both cases perhaps: "please ensure that this config symbols is described fully (less than $min_conf_desc_length lines is quite brief)" -apw ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] checkpatch.pl: Improve WARNING on Kconfig help 2018-12-19 11:59 ` Andy Whitcroft @ 2018-12-19 12:29 ` Joe Perches 2018-12-19 12:43 ` Igor Stoppa 2018-12-19 18:55 ` Igor Stoppa 0 siblings, 2 replies; 9+ messages in thread From: Joe Perches @ 2018-12-19 12:29 UTC (permalink / raw) To: Andy Whitcroft; +Cc: Igor Stoppa, igor.stoppa, linux-kernel On Wed, 2018-12-19 at 11:59 +0000, Andy Whitcroft wrote: > On Wed, Dec 19, 2018 at 02:44:36AM -0800, Joe Perches wrote: > > On Wed, 2018-12-19 at 10:35 +0200, Igor Stoppa wrote: > > > The checkpatch.pl script complains when the help section of a Kconfig > > > entry is too short, but it doesn't really explain what it is looking > > > for. Instead, it gives a generic warning that one should consider writing > > > a paragraph. > > > > > > But what it *really* checks is that the help section is at least > > > .$min_conf_desc_length lines long. > > > > > > Since the definition of what is a paragraph is not really carved in > > > stone (and actually the primary descriptions is "5 sentences"), make the > > > warning less ambiguous by expliciting the actual test condition, so that > > > one doesn't have to read checkpatch.pl sources, to figure out the actual > > > test. > > [] > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > [] > > > @@ -2931,7 +2931,8 @@ sub process { > > > } > > > if ($is_start && $is_end && $length < $min_conf_desc_length) { > > > WARN("CONFIG_DESCRIPTION", > > > - "please write a paragraph that describes the config symbol fully\n" . $herecurr); > > > + "please write a paragraph (" .$min_conf_desc_length . " lines)" . > > > > could say "(at least $min_conf_desc_length lines)" > > The original is better description in the semantic sense. We want them > to describe it well. We assume they haven't because it is short. We > don't want them to make it long, we want them to confirm it is fully > described. > > You arn't trying to make people make these warnings away, they should > just be checking they have met the criteria in the warning. If they > have they can ignore the warning and be happy, they don't have to add > two more lines. > > To cover both cases perhaps: > > "please ensure that this config symbols is described fully (less than > $min_conf_desc_length lines is quite brief)" This is one of those checkpatch bleats I never really thought was appropriate as some or many Kconfig symbols are fully descriptive in even with only a single line. Also, it seems you are arguing for a checkpatch --verbose-help output style rather than the intentionally terse single line output that the script produces today. That is something Al Viro once suggested in this thread: https://lore.kernel.org/patchwork/patch/775901/ On Sat, 2017-04-01 at 05:08 +0100, Al Viro wrote: > On Fri, Mar 31, 2017 at 08:52:50PM -0700, Joe Perches wrote: > > checkpatch messages are single line. > > Too bad... Incidentally, being able to get more detailed explanation of > a warning might be a serious improvement, especially if it contains > the rationale. Hell, something like TeX handling of errors might be > a good idea - warning printed, offered actions include 'give more help', > 'continue', 'exit', 'from now on suppress this kind of warning', 'from > now on just dump this kind of warning into log and keep going', 'from > now on dump all warnings into log and keep going'. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] checkpatch.pl: Improve WARNING on Kconfig help 2018-12-19 12:29 ` Joe Perches @ 2018-12-19 12:43 ` Igor Stoppa 2018-12-19 18:55 ` Igor Stoppa 1 sibling, 0 replies; 9+ messages in thread From: Igor Stoppa @ 2018-12-19 12:43 UTC (permalink / raw) To: Joe Perches, Andy Whitcroft; +Cc: igor.stoppa, linux-kernel On 19/12/2018 14:29, Joe Perches wrote: > On Wed, 2018-12-19 at 11:59 +0000, Andy Whitcroft wrote: >> On Wed, Dec 19, 2018 at 02:44:36AM -0800, Joe Perches wrote: >> To cover both cases perhaps: >> >> "please ensure that this config symbols is described fully (less than >> $min_conf_desc_length lines is quite brief)" > > This is one of those checkpatch bleats I never > really thought was appropriate as some or many > Kconfig symbols are fully descriptive in even > with only a single line. > > Also, it seems you are arguing for a checkpatch > --verbose-help output style rather than the > intentionally terse single line output that the > script produces today. If I have to use --verbose, to understand that the warning is about me writing 3 lines when the script expects 4, I don't think it's particularly user friendly. Let's write "Expected 4+ lines" or something equally clear. It will fit in a row and get the job done. > That is something Al Viro once suggested in this thread: > https://lore.kernel.org/patchwork/patch/775901/ > > On Sat, 2017-04-01 at 05:08 +0100, Al Viro wrote: >> On Fri, Mar 31, 2017 at 08:52:50PM -0700, Joe Perches wrote: >>> checkpatch messages are single line. >> >> Too bad... Incidentally, being able to get more detailed explanation of >> a warning might be a serious improvement, especially if it contains >> the rationale. Hell, something like TeX handling of errors might be >> a good idea - warning printed, offered actions include 'give more help', >> 'continue', 'exit', 'from now on suppress this kind of warning', 'from >> now on just dump this kind of warning into log and keep going', 'from >> now on dump all warnings into log and keep going'. It's all good in general, but here the word "paragraph" is being abused, in the sense that it has been given an arbitrary meaning of "4 lines". And the warning is even worse because it doesn't even acknowledge that I wrote something, even if it's a meager 1 or 2 lines. Which is even more confusing. As user, if I'm running checkpatch.pl and I get a warning, I should spend my time trying to decide if/how to fix it, not re-invoking it with extra options or reading its sources. -- igor ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] checkpatch.pl: Improve WARNING on Kconfig help 2018-12-19 12:29 ` Joe Perches 2018-12-19 12:43 ` Igor Stoppa @ 2018-12-19 18:55 ` Igor Stoppa 2018-12-19 19:17 ` Joe Perches 1 sibling, 1 reply; 9+ messages in thread From: Igor Stoppa @ 2018-12-19 18:55 UTC (permalink / raw) To: Andy Whitcroft, Joe Perches; +Cc: igor.stoppa, linux-kernel The checkpatch.pl script complains when the help section of a Kconfig entry is too short, but it doesn't really explain what it is looking for. Instead, it gives a generic warning that one should consider writing a paragraph. But what it *really* checks is that the help section is at least .$min_conf_desc_length lines long. Since the definition of what is a paragraph is not really carved in stone (and actually the primary descriptions is "5 sentences"), make the warning less ambiguous by expliciting the actual test condition, so that one doesn't have to read checkpatch.pl sources, to figure out the actual test. Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com> CC: Andy Whitcroft <apw@canonical.com> CC: Joe Perches <joe@perches.com> CC: linux-kernel@vger.kernel.org --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c883ec55654f..33568d7e28d1 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2931,7 +2931,7 @@ sub process { } if ($is_start && $is_end && $length < $min_conf_desc_length) { WARN("CONFIG_DESCRIPTION", - "please write a paragraph that describes the config symbol fully\n" . $herecurr); + "expecting a 'help' section of " .$min_conf_desc_length . "+ lines\n" . $herecurr); } #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } -- 2.19.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] checkpatch.pl: Improve WARNING on Kconfig help 2018-12-19 18:55 ` Igor Stoppa @ 2018-12-19 19:17 ` Joe Perches 2018-12-19 19:39 ` Andi Kleen 2018-12-19 23:23 ` Igor Stoppa 0 siblings, 2 replies; 9+ messages in thread From: Joe Perches @ 2018-12-19 19:17 UTC (permalink / raw) To: Igor Stoppa, Andy Whitcroft, Andi Kleen; +Cc: igor.stoppa, linux-kernel On Wed, 2018-12-19 at 20:55 +0200, Igor Stoppa wrote: > The checkpatch.pl script complains when the help section of a Kconfig > entry is too short, but it doesn't really explain what it is looking > for. Instead, it gives a generic warning that one should consider writing > a paragraph. > > But what it *really* checks is that the help section is at least > .$min_conf_desc_length lines long. > > Since the definition of what is a paragraph is not really carved in > stone (and actually the primary descriptions is "5 sentences"), make the > warning less ambiguous by expliciting the actual test condition, so that > one doesn't have to read checkpatch.pl sources, to figure out the actual > test. [] > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -2931,7 +2931,7 @@ sub process { > } > if ($is_start && $is_end && $length < $min_conf_desc_length) { > WARN("CONFIG_DESCRIPTION", > - "please write a paragraph that describes the config symbol fully\n" . $herecurr); > + "expecting a 'help' section of " .$min_conf_desc_length . "+ lines\n" . $herecurr); this could also be written without the concatenations "expecting a 'help' section of $min_conf_desc_length or more lines\n" . $herecurr); or maybe "please write a paragraph that describes the config symbol fully ($min_conf_desc_length or more lines)\n" . $herecurr); Andi? You are the originator of this text. Do you have an opinion? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] checkpatch.pl: Improve WARNING on Kconfig help 2018-12-19 19:17 ` Joe Perches @ 2018-12-19 19:39 ` Andi Kleen 2018-12-19 23:23 ` Igor Stoppa 1 sibling, 0 replies; 9+ messages in thread From: Andi Kleen @ 2018-12-19 19:39 UTC (permalink / raw) To: Joe Perches; +Cc: Igor Stoppa, Andy Whitcroft, igor.stoppa, linux-kernel > "expecting a 'help' section of $min_conf_desc_length or more lines\n" . $herecurr); > or maybe > "please write a paragraph that describes the config symbol fully ($min_conf_desc_length or more lines)\n" . $herecurr); > > Andi? > You are the originator of this text. > Do you have an opinion? Either change is fine for me. -Andi > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] checkpatch.pl: Improve WARNING on Kconfig help 2018-12-19 19:17 ` Joe Perches 2018-12-19 19:39 ` Andi Kleen @ 2018-12-19 23:23 ` Igor Stoppa 1 sibling, 0 replies; 9+ messages in thread From: Igor Stoppa @ 2018-12-19 23:23 UTC (permalink / raw) To: Andy Whitcroft, Joe Perches, Andi Kleen; +Cc: igor.stoppa, linux-kernel The checkpatch.pl script complains when the help section of a Kconfig entry is too short, but it doesn't really explain what it is looking for. Instead, it gives a generic warning that one should consider writing a paragraph. But what it *really* checks is that the help section is at least .$min_conf_desc_length lines long. Since the definition of what is a paragraph is not really carved in stone (and actually the primary descriptions is "5 sentences"), make the warning less ambiguous by expliciting the actual test condition, so that one doesn't have to read checkpatch.pl sources, to figure out the actual test. Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com> CC: Andy Whitcroft <apw@canonical.com> CC: Joe Perches <joe@perches.com> CC: Andi Kleen <ak@linux.intel.com> CC: linux-kernel@vger.kernel.org --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c883ec55654f..818ddada28b5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2931,7 +2931,7 @@ sub process { } if ($is_start && $is_end && $length < $min_conf_desc_length) { WARN("CONFIG_DESCRIPTION", - "please write a paragraph that describes the config symbol fully\n" . $herecurr); + "expecting a 'help' section of $min_conf_desc_length or more lines\n" . $herecurr); } #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } -- 2.19.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-12-19 23:24 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-19 8:35 [PATCH] checkpatch.pl: Improve WARNING on Kconfig help Igor Stoppa 2018-12-19 10:44 ` Joe Perches 2018-12-19 11:59 ` Andy Whitcroft 2018-12-19 12:29 ` Joe Perches 2018-12-19 12:43 ` Igor Stoppa 2018-12-19 18:55 ` Igor Stoppa 2018-12-19 19:17 ` Joe Perches 2018-12-19 19:39 ` Andi Kleen 2018-12-19 23:23 ` Igor Stoppa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).