From: Bill Fischofer [mailto:bill.fischo...@linaro.org] Sent: Tuesday, May 23, 2017 3:22 PM To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia.com> Cc: Maxim Uvarov <maxim.uva...@linaro.org>; lng-odp@lists.linaro.org Subject: Re: [lng-odp] [PATCHv2] scripts: checkpatch: update to allow additional exceptions
On Tue, May 23, 2017 at 7:07 AM, Savolainen, Petri (Nokia - FI/Espoo) <mailto:petri.savolai...@nokia.com> wrote: > -----Original Message----- > From: lng-odp [mailto:mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of > Maxim > Uvarov > Sent: Monday, May 22, 2017 10:07 PM > To: mailto:lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCHv2] scripts: checkpatch: update to allow > additional exceptions > > Merged! > > Maxim. > > On 05/04/17 22:33, Bill Fischofer wrote: > > Update http://checkpatch.pl to avoid issuing warnings for use of externs, > > volatile, or camelCase. > > > > Signed-off-by: Bill Fischofer <mailto:bill.fischo...@linaro.org> > > --- > > scripts/http://checkpatch.pl | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/http://checkpatch.pl b/scripts/http://checkpatch.pl > > index 16316b92..1c27ac60 100755 > > --- a/scripts/http://checkpatch.pl > > +++ b/scripts/http://checkpatch.pl > > @@ -4273,7 +4273,7 @@ sub process { > > > $camelcase_file_seeded = 1; > > } > > } > > - if (!defined > $camelcase{$word}) { > > + if (!defined > $camelcase{$word} && 0) { First, I think it's not good to edit the http://checkpatch.pl itself. We should just use the config file to document what checks are ignored. Also these direct edits are lost when we upgrade to new checkpatch version. Also, I think camel case check is useful. We are forced to use camel case sometimes due to external lib (openSSL) API, but those are exceptions and should be handled as such. Now this edit opens door for every patch to contain camel case, also when there's no reason to do so. We need reviewers to check for it now, which is a waste. So, I'd suggest to revert this. The camel case rule we want is very simple: you may use camel case symbols but you may not define any new ones. I don't believe checkpatch is set up to support that rule, however. Is there a way of doing this to your knowledge? [petri] Just the way we have done it so far: checkpatch warns on all camel cases and reviewers pass only those that have legitimate reasoning (== external lib usage) . -Petri
