[PATCH] checkpatch: Add check for too short Kconfig descriptions

[PATCH] checkpatch: Add check for too short Kconfig descriptions

[Andi Kleen]

Author: Andi Kleen <ak@linux.intel.com>
Date:   Sat Mar 20 00:26:42 2010 +0100

    checkpatch: Add check for too short Kconfig descriptions
    
    I've seen various new Kconfigs with rather unhelpful one liner
    descriptions. Add a Kconfig warning for a minimum length of the
    Kconfig help section.

    Right now I arbitarily chose 4. The exact value can be debated.
    
    Signed-off-by: Andi Kleen <ak@linux.intel.com>

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a4d7434..d1d36e9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1382,6 +1382,21 @@ sub process {
 			ERROR("trailing whitespace\n" . $herevet);
 		}
 
+# check for Kconfig help text having a real description
+		if ($realfile =~ /Kconfig/ && 
+		    $line =~ /\+?\s*(---)?help(---)?$/) {
+			my $length = 0;
+			for (my $l = $linenr; defined($lines[$l]); $l++) { 
+				my $f = $lines[$l];
+				$f =~ s/#.*//; 
+				$f =~ s/^\s+//;
+				next if ($f =~ /^$/);
+				last if ($f =~ /^\s*config\s/); 
+				$length++;
+			}
+			WARN("please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($length < 4);
+		}
+
 # check we are in a valid source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/);
 

Re: [PATCH] checkpatch: Add check for too short Kconfig descriptions

[Tilman Schmidt]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2010-03-20 03:32 schrieb Andi Kleen:
>     I've seen various new Kconfigs with rather unhelpful one liner
>     descriptions. Add a Kconfig warning for a minimum length of the
>     Kconfig help section.

I don't think helpfulness can be enforced via line count.
A one-liner can be quite sufficient (eg. CRYPTO_MD4).
OTOH more lines can still leave important questions unanswered (eg.
CRYPTO_SALSA20_586 whose help text is identical to that of
CRYPTO_SALSA20, leaving you wondering when to choose one over the other).

Jm2c
Tilman

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.12 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkukzjIACgkQQ3+did9BuFvE1wCff7n2x/bAbiZrJkTnQrEUi7eI
oVsAoJO6l5JMBddFfQOwQDetun1tCtEx
=qkDo
-----END PGP SIGNATURE-----

Re: [PATCH] checkpatch: Add check for too short Kconfig descriptions

[Andi Kleen]

On Sat, Mar 20, 2010 at 02:31:30PM +0100, Tilman Schmidt wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Am 2010-03-20 03:32 schrieb Andi Kleen:
> >     I've seen various new Kconfigs with rather unhelpful one liner
> >     descriptions. Add a Kconfig warning for a minimum length of the
> >     Kconfig help section.
> 
> I don't think helpfulness can be enforced via line count.

The warning merely is intended to get people to think
about that. Yes it cannot enforce it directly.

> A one-liner can be quite sufficient (eg. CRYPTO_MD4).

I don't think that one liner is sufficient. Consider the stand point
of someone who doesn't know anything of cryptography. They will
need at least one or two sentences to decided if they need
that option not.

Yes it will likely lead to some duplicate descriptions, but that's not
a problem. In fact it's a feature because it makes the job of
the person setting that option easier.

> OTOH more lines can still leave important questions unanswered (eg.
> CRYPTO_SALSA20_586 whose help text is identical to that of
> CRYPTO_SALSA20, leaving you wondering when to choose one over the other).

Yes it's not a perfect measure and can be circumvented. But hopefully
most users would not.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] checkpatch: Add check for too short Kconfig descriptions

[Tilman Schmidt]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2010-03-20 15:07 schrieb Andi Kleen:
> The warning merely is intended to get people to think
> about that. Yes it cannot enforce it directly.

Then change the wording, at least. With the current wording, people will
think, "But I did!", and complain about a false positive on LKML, where
they will be annoyed to learn that checkpatch.pl's criterion for
"describing fully" is "having at least four lines".

But even with a better wording, I think the warning will still do more
harm than good.

> Yes it's not a perfect measure and can be circumvented. But hopefully
> most users would not.

I'm not thinking of circumvention, but of well-meaning authors writing
long explanations that describe everything the author found worth
mentioning, but still don't answer the essential question: "Should I
select that option?" In fact, most of the unhelpful Kconfig help texts
I've encountered where longer than four lines. :-)

Thanks,
Tilman
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.12 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkulBN0ACgkQQ3+did9BuFtg4QCeNBnPAbMg3+dYfoimlG5/lQZ/
vRYAniohcXcoeC3+59eKSWwZ3B4fJ6Ix
=E5sA
-----END PGP SIGNATURE-----

Re: [PATCH] checkpatch: Add check for too short Kconfig descriptions

[Andi Kleen]

On Sat, Mar 20, 2010 at 06:24:45PM +0100, Tilman Schmidt wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Am 2010-03-20 15:07 schrieb Andi Kleen:
> > The warning merely is intended to get people to think
> > about that. Yes it cannot enforce it directly.
> 
> Then change the wording, at least. With the current wording, people will
> think, "But I did!", and complain about a false positive on LKML, where
> they will be annoyed to learn that checkpatch.pl's criterion for
> "describing fully" is "having at least four lines".

Change to what?  If you have a better suggestion I can change it.

> 
> But even with a better wording, I think the warning will still do more
> harm than good.

We have to agree to disagree on that then.


> 
> > Yes it's not a perfect measure and can be circumvented. But hopefully
> > most users would not.
> 
> I'm not thinking of circumvention, but of well-meaning authors writing
> long explanations that describe everything the author found worth
> mentioning, but still don't answer the essential question: "Should I
> select that option?" In fact, most of the unhelpful Kconfig help texts
> I've encountered where longer than four lines. :-)

I don't disagree that longer help texts can be unhelpful too,
but at least there's some chance that they are. 

For a single sentence it's very unlikely ever that it's helpful.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] checkpatch: Add check for too short Kconfig descriptions

[Tilman Schmidt]

-----BEGIN PGP SIGNED MESSAGE-----

Hash: SHA1



Am 2010-03-20 19:03 schrieb Andi Kleen:
> On Sat, Mar 20, 2010 at 06:24:45PM +0100, Tilman Schmidt wrote:
>> Am 2010-03-20 15:07 schrieb Andi Kleen:
>>> The warning merely is intended to get people to think
>>> about that. Yes it cannot enforce it directly.
>>
>> Then change the wording, at least. With the current wording, people will
>> think, "But I did!", and complain about a false positive on LKML, where
>> they will be annoyed to learn that checkpatch.pl's criterion for
>> "describing fully" is "having at least four lines".
> 
> Change to what?  If you have a better suggestion I can change it.

My suggestion:

- -+			WARN("please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($length < 4);
++			WARN("Less than four lines of help text -- consider adding more detail\n" . $herecurr) if ($length < 4);

>>
>> I'm not thinking of circumvention, but of well-meaning authors writing
>> long explanations that describe everything the author found worth
>> mentioning, but still don't answer the essential question: "Should I
>> select that option?" In fact, most of the unhelpful Kconfig help texts
>> I've encountered where longer than four lines. :-)
> 
> I don't disagree that longer help texts can be unhelpful too,
> but at least there's some chance that they are. 
> 
> For a single sentence it's very unlikely ever that it's helpful.

We have to agree to disagree on that then, too.

Regards,
Tilman
-----BEGIN PGP SIGNATURE-----

Version: GnuPG v2.0.12 (MingW32)

Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/



iEYEARECAAYFAkuqBZgACgkQQ3+did9BuFtgjwCgim0E0xnYJEnbCJsvKflkaSdk

ITQAn2zjFpo4ODhhpDvgJGMIaUWJnLgZ

=SMok

-----END PGP SIGNATURE-----

Re: [PATCH] checkpatch: Add check for too short Kconfig descriptions

[Andi Kleen]

> - -+			WARN("please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($length < 4);
> ++			WARN("Less than four lines of help text -- consider adding more detail\n" . $herecurr) if ($length < 4);

I don't want to give the number, because that just tempts people
to "game" the rule. Ideally they should just use as many lines
as needed, not just four.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] checkpatch: Add check for too short Kconfig descriptions

[Tilman Schmidt]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

Am 24.03.2010 14:39 schrieb Andi Kleen:
>> - -+			WARN("please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($length < 4);
>> ++			WARN("Less than four lines of help text -- consider adding more detail\n" . $herecurr) if ($length < 4);
> 
> I don't want to give the number, because that just tempts people
> to "game" the rule. Ideally they should just use as many lines
> as needed, not just four.

You're working against people instead of with them.
IMHO that is never a good idea.

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]