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 C75851BA51 for ; Thu, 10 May 2018 13:45:04 +0200 (CEST) To: Jerin Jacob Cc: dev@dpdk.org References: <152591991920.119328.14523975619615362920.stgit@localhost.localdomain> <20180510061731.GB12718@jerin> <20180510091147.GA26838@jerin> From: Andy Green Message-ID: Date: Thu, 10 May 2018 19:44:34 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 In-Reply-To: <20180510091147.GA26838@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 11:45:04 -0000 On 05/10/2018 05:11 PM, Jerin Jacob wrote: > -----Original Message----- >> Date: Thu, 10 May 2018 14:46:42 +0800 >> From: Andy Green >> To: Jerin Jacob >> CC: dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH v3 00/40] Fix build on gcc8 and various bugs >> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 >> Thunderbird/52.7.0 >> >> >> >> 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 >>>> >>> ./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? > > I think, the underscore check is to make sure that the subject should not have > C symbols. Right, that seems to be the intention. But if the patch is entirely about doing something to a specific C symbol or function, it's not a bad thing if it mentions that in the title is it? In itself, most projects would consider it a good thing to concisely explain what it's doing. Eg, "fix NULL pointer exception in my_function if unconfigured" is illegal for this project. It's strange actually. I don't understand what negative outcome the check is saving us from. If nothing, maybe it should be patched to not do that any more. > Change to following will work: > > net/nfp: fix buffer overflow Sure, I studied the script to find out what its problem was. I just don't think its problem is reasonable. If nobody cares, sure I will go through removing useful information from my patch titles to keep this script happy. -Andy >> >> -Andy