>> I think it's preferable to diasble the check here. The CIPHEr names use '_'; >> it would be less readable to remove them >>testDecryptDES_CBC_MD4_1 is way better than testDecryptDesCbcMD41
Good catch here! Just wonder if it's doable to only disable the check here like we can in PMD. -----Original Message----- From: Emmanuel Lécharny [mailto:[email protected]] Sent: Sunday, July 05, 2015 3:38 PM To: [email protected] Subject: Re: Kerby coding style Le 04/07/15 16:40, Stefan Seelmann a écrit : > 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 Obviously (1) > > > ParameterName: > Same as above (LocalVariableName) > Poll: > (1) change parameter names > (2) disable check (1) too > > > MethodName: > Some test methods contain underscores (e.g. > DecryptionTest.testDecryptDES_CBC_MD4_1) > Poll: > (1) change method names > (2) disable check I think it's preferable to diasble the check here. The CIPHEr names use '_'; it would be less readable to remove them testDecryptDES_CBC_MD4_1 is way better than testDecryptDesCbcMD41 > > > 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 (1) > > > 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 (2) : more readable, remove potential error when those conditional expressions are changed. > > > LineLength: > Poll: > (1) allow line length of 120 > (2) force line length of 80 120 > > > Whitespace: > There are about 1100 whitespace violations > Poll: > (1) follow Sun/Oracle Java convention (requires reformatting with > formatter, see below) > (2) something else (1) > > > Curly braces: > There are 89 violations > Poll: > (1) follow Sun/Oracle Java convention (requires reformatting with > formatter, see below) > (2) something else (1) > > > 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 (1) Thanks Stefan!
