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:[email protected]] Sent: Saturday, July 04, 2015 10:41 PM To: [email protected] 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
