From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.warmcat.com (mail.warmcat.com [163.172.24.82]) by dpdk.org (Postfix) with ESMTP id 108F57CBE for ; Thu, 10 May 2018 08:46:55 +0200 (CEST) To: Jerin Jacob Cc: dev@dpdk.org References: <152591991920.119328.14523975619615362920.stgit@localhost.localdomain> <20180510061731.GB12718@jerin> From: Andy Green Message-ID: Date: Thu, 10 May 2018 14:46:42 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 In-Reply-To: <20180510061731.GB12718@jerin> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 00/40] Fix build on gcc8 and various bugs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 May 2018 06:46:55 -0000 On 05/10/2018 02:17 PM, Jerin Jacob wrote: > -----Original Message----- >> Date: Thu, 10 May 2018 10:46:18 +0800 >> From: Andy Green >> To: dev@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