+1 for Lenni's proposed coding style; also +1 for Colin's proposal of integrating an automatic tool into sentry code base. whenever run maven, the code can be validated through such tool. Not familiar with FindBugs plugin if it can be integrated into maven. Maybe can open another voting thread for any suggested style tool?
Best, Anne On Fri, Oct 16, 2015 at 1:33 AM, Sun, Dapeng <[email protected]> wrote: > +1 for the voting. > > > Regards > Dapeng > > -----Original Message----- > From: Ma, Junjie [mailto:[email protected]] > Sent: Friday, October 16, 2015 4:18 PM > To: [email protected] > Subject: RE: [VOTE] Adopt Sentry Project Coding Style Guide and Best > Practices > > +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 > -- Thanks, Anne
