directory-kerby mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zheng, Kai" <kai.zh...@intel.com>
Subject RE: Kerby coding style
Date Sat, 04 Jul 2015 23:36:01 GMT
Hi Stefan,

Thanks for the hard work and sorting this great list out!! 
I'm OK with your preferred options, because most time they're also mine. I'm surprised we
have so many exceptions! Just one thing, LineLength. I'm OK to use 120 for the check, but
I would suggest we should try to avoid so long lines. Yes, we should never exceed the 120
limit, and if you do, it will fail the building; meanwhile, 80 is encouraged to make it consistent
with most of existing codes. I don't wish to argue here why we choose 120 over 80, or otherwise,
because it's hard. Many discussion about this occurred elsewhere, like Hadoop community; we
would also, but I don't think we have the time. I would rather avoid lengthy discussions for
such things and remain our limited energy for the codes. Would this work? Thanks.

Do we need a branch enforcing the check right now so all the guys can help? So many exceptions
... Thanks again for the taking!

Regards,
Kai

-----Original Message-----
From: Stefan Seelmann [mailto:mail@stefan-seelmann.de] 
Sent: Saturday, July 04, 2015 10:41 PM
To: kerby@directory.apache.org
Subject: Kerby coding style

Hi all,

I created a basic checkstyle config [1] and fixed some (to me) obvious issues. Some more checks
make sense which require more changes in the code but I'd like to ask all developers for their
opinions. Once we reach consensus I'd release the checkstyle file and add the check to the
build, in the beginning as deactivated profile or non-failing, once all issues are fixed activated
by default.


So here as quick poll, please express your opinion, my preferred option is always (1) ;).


LocalVariableName:
Sometimes variable names like 'W1' (Asn1String) or 'Kc' (KcCheckSum) are used. However variable
names should start with lowercase character. I guess this was done to follow names in specifications.
Poll:
(1) change variable names
(2) disable check


ParameterName:
Same as above (LocalVariableName)
Poll:
(1) change parameter names
(2) disable check


MethodName:
Some test methods contain underscores (e.g.
DecryptionTest.testDecryptDES_CBC_MD4_1)
Poll:
(1) change method names
(2) disable check


AvoidStarImport:
There are many usages of '*' imports, like 'import org.ietf.jgss.*;'
Poll:
(1) force explicit imports or each class
(2) allow star imports and disalbe check


AvoidInlineConditionals
There are 45 inline conditionals (e.g. 'return s != null ?
s.getVersion() : -1;')
Poll:
(1) allow inline condidtionals and disable check
(2) refactor inline conditionals


LineLength:
Poll:
(1) allow line length of 120
(2) force line length of 80


Whitespace:
There are about 1100 whitespace violations
Poll:
(1) follow Sun/Oracle Java convention (requires reformatting with formatter, see below)
(2) something else


Curly braces:
There are 89 violations
Poll:
(1) follow Sun/Oracle Java convention (requires reformatting with formatter, see below)
(2) something else


Formatter:
For Eclipse I'd also create a formatter with the following settings:
* Based on Java Conventions with following changes
* 4 spaces, no tabs
* Line length 120 instead of 80
* No comment formatting (Javadoc, block, line)
Poll:
(1) that is fine
(2) something else


Kind Regards,
Stefan

[1]
https://svn.apache.org/repos/asf/directory/buildtools/checkstyle-configuration/trunk/src/main/resources/kerby-checks.xml

Mime
View raw message