+10 :-) I am in complete agreement with Mike. It is usually painful for the reviewer and the developer to go back and fix style issues in each review. I am not sure if a precommit hook will suffice, since we don't actually run pre-commit builds. We will probably need to add it to the full build so the developer can figure out the issues before even submitting the patch for review.
On Fri, Jun 24, 2016 at 11:09 PM Lior Zeno <[email protected]> wrote: > +1. > We had a thread about it here: > > http://mail-archives.apache.org/mod_mbox/flume-dev/201606.mbox/raw/%3CCAA6RhS9%2BzsJ8GNom3FSjB7MN_Zb2aWfOSXxh_RC-MuvhAfQC7g%40mail.gmail.com%3E/1 > In addition, I created a jira issue for that. I hope we can add this to > 1.8.0: https://issues.apache.org/jira/browse/FLUME-2937 > > On Sat, Jun 25, 2016 at 1:11 AM, Ashish <[email protected]> wrote: > > > +1 > > > > On Fri, Jun 24, 2016 at 2:24 PM, Mike Percy <[email protected]> wrote: > > > Hey devs, > > > Code nitpicks have come up a bit lately (in code I'm the reviewer of). > > > Other Apache projects such as HBase and Kafka use checkstyle to do a > > > pre-commit check at build time. Rather than spend time going back and > > forth > > > on code style, how about we adopt the checkstyle plugin for Flume? > > > > > > I'd propose adopting the Google Java style. It's what the vast > majority > > of > > > the Flume code uses today, and there is a config file shipped with > > > checkstyle for it. Here's a link to it: > > > https://google.github.io/styleguide/javaguide.html > > > > > > My goal is just to maintain a consistent style throughout the code base > > and > > > avoid the review noise. Please let me know whether or not this sounds > > > helpful. > > > > > > Thanks, > > > Mike > > > > > > > > -- > > thanks > > ashish > > > > Blog: http://www.ashishpaliwal.com/blog > > My Photo Galleries: http://www.pbase.com/ashishpaliwal > > >
