[dpdk-dev] [PATCH v3 00/40] Fix build on gcc8 and various bugs

[Andy Green]

On 05/10/2018 02:17 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Thu, 10 May 2018 10:46:18 +0800
>> From: Andy Green <andy at warmcat.com>
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v3 00/40] Fix build on gcc8 and various bugs
>> User-Agent: StGit/unknown-version
>>
>> The following series gets current master able to build
>> itself, and allow lagopus to build against it, on Fedora 28 +
>> x86_64 using gcc 8.0.1.
>>
>> The first 17 patches have already been through two spins and
>> this time are corrected for all the comment (thanks to
>> everybody who commented) since v2, and have tested-by /
>> acked-bys applied.  The first workaround patch for the hash
>> function cast problem is dropped since something has already
>> been applied in master since yesterday to address it.
>>
>> The additional 23 patches are fixes for problems found
>> actually trying to build lagopus using current master.
>> These are almost entirely related to signed / unsigned
>> or truncation without explicit casts inside dpdk
>> headers.
>>
>> ---
> 
> Please run the following scripts before submitting the patch. There are plenty of errors.
> ./devtools/checkpatches.sh

Actually I checked them already with kernel checkpatch.pl.

There is only one patch where the existing code I am patching does not 
pass checkpatch rules already, this kind of thing.

WARNING:LONG_LINE: line over 80 characters
#19: FILE: lib/librte_eal/common/include/arch/x86/rte_memcpy.h:600:
+    tmp = (int)len; 
                                     \

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#19: FILE: lib/librte_eal/common/include/arch/x86/rte_memcpy.h:600:
+    tmp = (int)len; 
                                     \$


So what should I better do when the project doesn't follow its own 
super-pedantic rules?  I chose to follow the formatting that was already 
there.

> ./devtools/check-git-log.sh

Ugh...

Wrong headline format:
	drivers/net/nfp: fix buffer overflow in fw_name

... snip something "wrong" about every patch title...

It's just doing this

# check headline format (spacing, no punctuation, no code)
bad=$(echo "$headlines" | grep --color=always \
         -e '    ' \
         -e '^ ' \
         -e ' $' \
         -e '\.$' \
         -e '[,;!?&|]' \
         -e ':.*_' \
         -e '^[^:]\+$' \
         -e ':[^ ]' \
         -e ' :' \
         | sed 's,^,\t,')
[ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"

It probably seems to whoever wrote it that adds "quality", but actually 
inflexible rules like this do nothing to help quality of the patch 
payload and actively put off contribution.

So on this first one it's hitting the rule ':.*_', ie, this project 
believes there should never be a patch title mentioning anything with an 
underscore after a colon.

Can you help me understand in what way banning mentioning relevant 
strings in the patch title is a good idea?  It's actively reducing the 
value of the patch title, isn't it?

-Andy