-----Original Message----- > Date: Thu, 10 May 2018 19:44:34 +0800 > From: Andy Green <a...@warmcat.com> > To: Jerin Jacob <jerin.ja...@caviumnetworks.com> > 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 <a...@warmcat.com> > > > To: Jerin Jacob <jerin.ja...@caviumnetworks.com> > > > 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 <a...@warmcat.com> > > > > > 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
