+1, and do we need to add FindBugs plugin to check the code? Best regards,
Colin Ma(Ma Jun Jie) -----Original Message----- From: Lenni Kuff [mailto:[email protected]] Sent: Friday, October 16, 2015 3:18 PM To: [email protected] Subject: [VOTE] Adopt Sentry Project Coding Style Guide and Best Practices Hi, As a follow up to this [1] discussion thread, I would like to propose a vote to adopt a formal project coding style guide and coding best practices for all new contributions. The style guide proposed is similar to the Hadoop project's style guide [2], with a few items removed that did not make sense for Sentry. I have attached the full details at the bottom of this mail. Please respond if you have any concerns or think we should add/remove/modify anything proposed. Vote will be opened for 72 hours. [ ] +1 approve [ ] +0 no opinion [ ] -1 disapprove (and reason why) Thanks, Lenni [1] - http://mail-archives.apache.org/mod_mbox/sentry-dev/201508.mbox/browser [2] - https://wiki.apache.org/hadoop/CodeReviewChecklist *Coding Style* This is a modified version of the Hadoop project's code style guide <https://wiki.apache.org/hadoop/CodeReviewChecklist>. *new code:* - follows Sun's code conventions <http://www.oracle.com/technetwork/java/javase/documentation/codeconvtoc-136057.html> except indentation is 2 spaces, not 4 *changes to existing code:* - maintains existing style Documentation *Internal javadoc:* - accurate, sufficient for future maintainability *Public API javadoc:* - accurate, sufficient for developers to code against - follows standard javadoc conventions - system properties, configuration options, and resources covered - illegal arguments are properly documented as appropriate - package and overview javadoc are updated as appropriate Coding Best Practices - implementation matches what the documentation says - logger name is effectively the result of Class.getName() - class & member access - as restricted as it can be (subject to testing requirements) - appropriate NullPointerException and IllegalArgumentException argument checks - look for accidental propagation of exceptions - look for unanticipated runtime exceptions - - try-finally used as necessary to restore consistent state - possible deadlocks - look for inconsistent locking order - race conditions - look for missing or inadequate synchronization - consistent synchronization - always locking the same object(s) - look for synchronization or documentation saying there's no synchronization - look for possible performance problems - look at boundary conditions for problems - configuration entries are retrieved/set via setter/getter methods - implementation details do NOT leak into interfaces - variables and arguments should be interfaces where possible - if equals is overridden then hashCode is overridden (and vice versa) - objects are checked (instanceof) for appropriate type before casting (use generics if possible) - public API changes have been publicly discussed - use of static member variables should be used with caution Tests - unit tests exist for bug fixes and new features, or a rationale is given in Jira for why there is no test
