Can we make sure there is a way to avoid running the automatic tool? I.e. either the tool is a separate command that the precommit/release process runs, or the tool runs under normal mvn compile/test but has a flag to disable?
I've seen a couple of cases where these things run while you are developing and it's pretty invasive. Greg On Fri, Oct 16, 2015 at 10:50 AM, Anne Yu <[email protected]> wrote: > +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 >
