From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v3 00/40] Fix build on gcc8 and various bugs Date: Thu, 10 May 2018 17:28:00 +0530 Message-ID: <20180510115759.GA8776@jerin> References: <152591991920.119328.14523975619615362920.stgit@localhost.localdomain> <20180510061731.GB12718@jerin> <20180510091147.GA26838@jerin> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: Andy Green Return-path: Received: from NAM01-BN3-obe.outbound.protection.outlook.com (mail-bn3nam01on0087.outbound.protection.outlook.com [104.47.33.87]) by dpdk.org (Postfix) with ESMTP id 652DF1B85E for ; Thu, 10 May 2018 13:58:22 +0200 (CEST) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" -----Original Message----- > Date: Thu, 10 May 2018 19:44:34 +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 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 think, the rational is # In subject you have minimal information # In commit log, you can have DETAILED info That translated to following in your example: module: fix a NULL pointer exception fix a NULL pointer exception in my_function if unconfigured due to so and so reason > > 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. IMO, Keep all useful information in description not in subject. > > -Andy > > > > > > > -Andy