You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@crunch.apache.org by Josh Wills <jw...@cloudera.com> on 2012/06/16 00:17:42 UTC

Re: Checkstyle & Reviews

I'm in favor of a style guide; consistent layout makes it easier for
my brain to devour code. That said, I'm not up-to-date on best
practices here in the real world; my only experience with automated
code review tools was at teh goog.

Re: code reviews, let's start that discussion up in another thread.

J

On Fri, Jun 15, 2012 at 3:13 PM, Robert Chu <ro...@wibidata.com> wrote:
> Hey Everybody,
>
> I'd like to start a discussion about using automated code review tools
> to improve the crunch development process. I'd I'm personally a big
> fan of using tools that can help improve code quality. These tools
> often include things like the maven-checkstyle-plugin and the maven-
> findbugs-plugin. I am currently unaware of any comparable scala
> automated tools. If people have opinions on whether or not we should
> use tools like this or which tools we should be using specifically
> please let us know.
>
> Also, another related question: Should we have some sort of a code
> review process?
>
> Robert Chu



-- 
Director of Data Science
Cloudera
Twitter: @josh_wills

Re: Checkstyle & Reviews

Posted by Gabriel Reid <ga...@gmail.com>.
Eclipse style config added to the project (under the res directory). 

I couldn't get maven integration to work (seems that it's not actively maintained), but that doesn't seem all bad as the maven integration was apparently only on workspace level anyhow (as opposed to being on project level). 

The profile that I've set up has indentation of two spaces, max line length of 100, all the rest is the default Eclipse settings.

- Gabriel


On Tuesday 19 June 2012 at 09:14, Gabriel Reid wrote:

> On Mon, Jun 18, 2012 at 5:07 PM, Josh Wills <jwills@cloudera.com (mailto:jwills@cloudera.com)> wrote:
> > Can we check in the Eclipse style config? And is it possible to
> > integrate it w/mvn eclipse:eclipse so that it automatically gets added
> > to the project? (I haven't used them before, obviously.)
> > 
> 
> 
> I'll add an Eclipse style config to the project that replicates the
> current style as closely as possible.
> 
> It appears that it's possible to set up code formatting rules on a
> workspace (and maybe project) basis from maven
> (http://maven.apache.org/plugins/maven-eclipse-plugin/examples/load-code-styles.html),
> but as usual I'm confused by maven plugin documentation. I'll try to
> figure out how to make this work as well so that "mvn eclipse:eclipse"
> will automatically set up the code formatting.
> 
> At the very least we'll end up with a style config that needs to be
> added to an Eclipse project/workspace just once, and in the best case
> scenario everything will Just Work.
> 
> - Gabriel 


Re: Checkstyle & Reviews

Posted by Gabriel Reid <ga...@gmail.com>.
On Mon, Jun 18, 2012 at 5:07 PM, Josh Wills <jw...@cloudera.com> wrote:
> Can we check in the Eclipse style config? And is it possible to
> integrate it w/mvn eclipse:eclipse so that it automatically gets added
> to the project? (I haven't used them before, obviously.)

I'll add an Eclipse style config to the project that replicates the
current style as closely as possible.

It appears that it's possible to set up code formatting rules on a
workspace (and maybe project) basis from maven
(http://maven.apache.org/plugins/maven-eclipse-plugin/examples/load-code-styles.html),
but as usual I'm confused by maven plugin documentation. I'll try to
figure out how to make this work as well so that "mvn eclipse:eclipse"
will automatically set up the code formatting.

At the very least we'll end up with a style config that needs to be
added to an Eclipse project/workspace just once, and in the best case
scenario everything will Just Work.

- Gabriel

Re: Checkstyle & Reviews

Posted by Patrick Hunt <ph...@apache.org>.
I'm not a pmc chair, Tom can do it though. Send him the list of apache
ids you want to allow job creation/editing (ie admin).

Patrick

On Mon, Jun 18, 2012 at 8:07 AM, Josh Wills <jw...@cloudera.com> wrote:
> +phunt
>
> http://wiki.apache.org/general/Jenkins
>
> I think Patrick can set us up with Jenkins.
>
> Can we check in the Eclipse style config? And is it possible to
> integrate it w/mvn eclipse:eclipse so that it automatically gets added
> to the project? (I haven't used them before, obviously.)
>
> J
>
> On Sun, Jun 17, 2012 at 11:42 PM, Christian Tzolov
> <ch...@gmail.com> wrote:
>> +1 for shared eclipse code style configuration
>>
>> IMO using code quality tools as maven plugins is inefficient and hard to
>> maintain. More consistent and complete solution would be use Hudson or
>> Jenkins. Not sure though if apache provides such CI tools?
>>
>>
>> On Sat, Jun 16, 2012 at 10:27 AM, Gabriel Reid <ga...@gmail.com>
>> wrote:
>>>
>>> +1 for a style guide and/or formatting rules and coding rules.
>>>
>>> For code style, something that I've seen work well (at least if everyone
>>> is using Eclipse) is to just have a shared code style definition that you
>>> can import into Eclipse, and then set Eclipse to apply that style whenever
>>> you save a file. This works great if everyone uses the same style
>>> definition, and breaks down rapidly if they don't.
>>>
>>> I don't have any experience using the maven-findbugs-plugin, but I do find
>>> the Checkstyle useful, as long as the settings are sane (which is not always
>>> the case).
>>>
>>> On Saturday 16 June 2012 at 00:17, Josh Wills wrote:
>>>
>>> > I'm in favor of a style guide; consistent layout makes it easier for
>>> > my brain to devour code. That said, I'm not up-to-date on best
>>> > practices here in the real world; my only experience with automated
>>> > code review tools was at teh goog.
>>> >
>>> > Re: code reviews, let's start that discussion up in another thread.
>>> >
>>> > J
>>> >
>>> > On Fri, Jun 15, 2012 at 3:13 PM, Robert Chu <robert@wibidata.com
>>> > (mailto:robert@wibidata.com)> wrote:
>>> > > Hey Everybody,
>>> > >
>>> > > I'd like to start a discussion about using automated code review tools
>>> > > to improve the crunch development process. I'd I'm personally a big
>>> > > fan of using tools that can help improve code quality. These tools
>>> > > often include things like the maven-checkstyle-plugin and the maven-
>>> > > findbugs-plugin. I am currently unaware of any comparable scala
>>> > > automated tools. If people have opinions on whether or not we should
>>> > > use tools like this or which tools we should be using specifically
>>> > > please let us know.
>>> > >
>>> > > Also, another related question: Should we have some sort of a code
>>> > > review process?
>>> > >
>>> > > Robert Chu
>>> >
>>> >
>>> >
>>> > --
>>> > Director of Data Science
>>> > Cloudera
>>> > Twitter: @josh_wills
>>>
>>>
>>
>
>
>
> --
> Director of Data Science
> Cloudera
> Twitter: @josh_wills

Re: Checkstyle & Reviews

Posted by Josh Wills <jw...@cloudera.com>.
+phunt

http://wiki.apache.org/general/Jenkins

I think Patrick can set us up with Jenkins.

Can we check in the Eclipse style config? And is it possible to
integrate it w/mvn eclipse:eclipse so that it automatically gets added
to the project? (I haven't used them before, obviously.)

J

On Sun, Jun 17, 2012 at 11:42 PM, Christian Tzolov
<ch...@gmail.com> wrote:
> +1 for shared eclipse code style configuration
>
> IMO using code quality tools as maven plugins is inefficient and hard to
> maintain. More consistent and complete solution would be use Hudson or
> Jenkins. Not sure though if apache provides such CI tools?
>
>
> On Sat, Jun 16, 2012 at 10:27 AM, Gabriel Reid <ga...@gmail.com>
> wrote:
>>
>> +1 for a style guide and/or formatting rules and coding rules.
>>
>> For code style, something that I've seen work well (at least if everyone
>> is using Eclipse) is to just have a shared code style definition that you
>> can import into Eclipse, and then set Eclipse to apply that style whenever
>> you save a file. This works great if everyone uses the same style
>> definition, and breaks down rapidly if they don't.
>>
>> I don't have any experience using the maven-findbugs-plugin, but I do find
>> the Checkstyle useful, as long as the settings are sane (which is not always
>> the case).
>>
>> On Saturday 16 June 2012 at 00:17, Josh Wills wrote:
>>
>> > I'm in favor of a style guide; consistent layout makes it easier for
>> > my brain to devour code. That said, I'm not up-to-date on best
>> > practices here in the real world; my only experience with automated
>> > code review tools was at teh goog.
>> >
>> > Re: code reviews, let's start that discussion up in another thread.
>> >
>> > J
>> >
>> > On Fri, Jun 15, 2012 at 3:13 PM, Robert Chu <robert@wibidata.com
>> > (mailto:robert@wibidata.com)> wrote:
>> > > Hey Everybody,
>> > >
>> > > I'd like to start a discussion about using automated code review tools
>> > > to improve the crunch development process. I'd I'm personally a big
>> > > fan of using tools that can help improve code quality. These tools
>> > > often include things like the maven-checkstyle-plugin and the maven-
>> > > findbugs-plugin. I am currently unaware of any comparable scala
>> > > automated tools. If people have opinions on whether or not we should
>> > > use tools like this or which tools we should be using specifically
>> > > please let us know.
>> > >
>> > > Also, another related question: Should we have some sort of a code
>> > > review process?
>> > >
>> > > Robert Chu
>> >
>> >
>> >
>> > --
>> > Director of Data Science
>> > Cloudera
>> > Twitter: @josh_wills
>>
>>
>



-- 
Director of Data Science
Cloudera
Twitter: @josh_wills

Re: Checkstyle & Reviews

Posted by Christian Tzolov <ch...@gmail.com>.
+1 for shared eclipse code style configuration

IMO using code quality tools as maven plugins is inefficient and hard to
maintain. More consistent and complete solution would be use Hudson or
Jenkins. Not sure though if apache provides such CI tools?


On Sat, Jun 16, 2012 at 10:27 AM, Gabriel Reid <ga...@gmail.com>wrote:

> +1 for a style guide and/or formatting rules and coding rules.
>
> For code style, something that I've seen work well (at least if everyone
> is using Eclipse) is to just have a shared code style definition that you
> can import into Eclipse, and then set Eclipse to apply that style whenever
> you save a file. This works great if everyone uses the same style
> definition, and breaks down rapidly if they don't.
>
> I don't have any experience using the maven-findbugs-plugin, but I do find
> the Checkstyle useful, as long as the settings are sane (which is not
> always the case).
>
> On Saturday 16 June 2012 at 00:17, Josh Wills wrote:
>
> > I'm in favor of a style guide; consistent layout makes it easier for
> > my brain to devour code. That said, I'm not up-to-date on best
> > practices here in the real world; my only experience with automated
> > code review tools was at teh goog.
> >
> > Re: code reviews, let's start that discussion up in another thread.
> >
> > J
> >
> > On Fri, Jun 15, 2012 at 3:13 PM, Robert Chu <robert@wibidata.com(mailto:
> robert@wibidata.com)> wrote:
> > > Hey Everybody,
> > >
> > > I'd like to start a discussion about using automated code review tools
> > > to improve the crunch development process. I'd I'm personally a big
> > > fan of using tools that can help improve code quality. These tools
> > > often include things like the maven-checkstyle-plugin and the maven-
> > > findbugs-plugin. I am currently unaware of any comparable scala
> > > automated tools. If people have opinions on whether or not we should
> > > use tools like this or which tools we should be using specifically
> > > please let us know.
> > >
> > > Also, another related question: Should we have some sort of a code
> > > review process?
> > >
> > > Robert Chu
> >
> >
> >
> > --
> > Director of Data Science
> > Cloudera
> > Twitter: @josh_wills
>
>
>

Re: Checkstyle & Reviews

Posted by Christian Tzolov <ch...@gmail.com>.
If the discussion is just about using the codestyle then the following
comments are irrelevant.

Waiving code quality tools (like Checkstyle , PMD or Findings) "manually"
as maven  pluggings is usually inefficient and inconsistent compared to
tools like: Jenkins + Sonar. Sonar integrates all important quality tools
out of the box, provides consistent metrics, reports and management.
Apache provides Jenkins not sure about Sonar?
Having said this I don't mind using either approach. It is the code that
matters and who is looking at it ;)
Cheers,
Chris

 On Jun 21, 2012 7:34 AM, "Robert Chu" <ro...@wibidata.com> wrote:

> Using checkstyle with maven has worked well for me in my experience as
> well. For those of you who have had a bad experience with the
> maven-checkstyle combination, what went wrong? I'd like to have a
> checkstyle system that doesn't require developers to use eclipse (I don't
> use eclipse for development).
>
> On Tue, Jun 19, 2012 at 2:01 PM, Tom White <to...@cloudera.com> wrote:
>
>> On Sat, Jun 16, 2012 at 3:27 AM, Gabriel Reid <ga...@gmail.com>
>> wrote:
>> > +1 for a style guide and/or formatting rules and coding rules.
>> >
>> > For code style, something that I've seen work well (at least if
>> everyone is using Eclipse) is to just have a shared code style definition
>> that you can import into Eclipse, and then set Eclipse to apply that style
>> whenever you save a file. This works great if everyone uses the same style
>> definition, and breaks down rapidly if they don't.
>> >
>> > I don't have any experience using the maven-findbugs-plugin, but I do
>> find the Checkstyle useful, as long as the settings are sane (which is not
>> always the case).
>>
>> We've had a good experience with Checkstyle in Apache Whirr. The way
>> we have it set up is to fail the package build if there are Checkstyle
>> errors. The relevant POM for doing that is at
>> http://svn.apache.org/repos/asf/whirr/trunk/pom.xml
>>
>> Tom
>>
>> >
>> > On Saturday 16 June 2012 at 00:17, Josh Wills wrote:
>> >
>> >> I'm in favor of a style guide; consistent layout makes it easier for
>> >> my brain to devour code. That said, I'm not up-to-date on best
>> >> practices here in the real world; my only experience with automated
>> >> code review tools was at teh goog.
>> >>
>> >> Re: code reviews, let's start that discussion up in another thread.
>> >>
>> >> J
>> >>
>> >> On Fri, Jun 15, 2012 at 3:13 PM, Robert Chu <robert@wibidata.com(mailto:
>> robert@wibidata.com)> wrote:
>> >> > Hey Everybody,
>> >> >
>> >> > I'd like to start a discussion about using automated code review
>> tools
>> >> > to improve the crunch development process. I'd I'm personally a big
>> >> > fan of using tools that can help improve code quality. These tools
>> >> > often include things like the maven-checkstyle-plugin and the maven-
>> >> > findbugs-plugin. I am currently unaware of any comparable scala
>> >> > automated tools. If people have opinions on whether or not we should
>> >> > use tools like this or which tools we should be using specifically
>> >> > please let us know.
>> >> >
>> >> > Also, another related question: Should we have some sort of a code
>> >> > review process?
>> >> >
>> >> > Robert Chu
>> >>
>> >>
>> >>
>> >> --
>> >> Director of Data Science
>> >> Cloudera
>> >> Twitter: @josh_wills
>> >
>> >
>>
>
>

Re: Checkstyle & Reviews

Posted by Robert Chu <ro...@wibidata.com>.
Using checkstyle with maven has worked well for me in my experience as
well. For those of you who have had a bad experience with the
maven-checkstyle combination, what went wrong? I'd like to have a
checkstyle system that doesn't require developers to use eclipse (I don't
use eclipse for development).

On Tue, Jun 19, 2012 at 2:01 PM, Tom White <to...@cloudera.com> wrote:

> On Sat, Jun 16, 2012 at 3:27 AM, Gabriel Reid <ga...@gmail.com>
> wrote:
> > +1 for a style guide and/or formatting rules and coding rules.
> >
> > For code style, something that I've seen work well (at least if everyone
> is using Eclipse) is to just have a shared code style definition that you
> can import into Eclipse, and then set Eclipse to apply that style whenever
> you save a file. This works great if everyone uses the same style
> definition, and breaks down rapidly if they don't.
> >
> > I don't have any experience using the maven-findbugs-plugin, but I do
> find the Checkstyle useful, as long as the settings are sane (which is not
> always the case).
>
> We've had a good experience with Checkstyle in Apache Whirr. The way
> we have it set up is to fail the package build if there are Checkstyle
> errors. The relevant POM for doing that is at
> http://svn.apache.org/repos/asf/whirr/trunk/pom.xml
>
> Tom
>
> >
> > On Saturday 16 June 2012 at 00:17, Josh Wills wrote:
> >
> >> I'm in favor of a style guide; consistent layout makes it easier for
> >> my brain to devour code. That said, I'm not up-to-date on best
> >> practices here in the real world; my only experience with automated
> >> code review tools was at teh goog.
> >>
> >> Re: code reviews, let's start that discussion up in another thread.
> >>
> >> J
> >>
> >> On Fri, Jun 15, 2012 at 3:13 PM, Robert Chu <robert@wibidata.com(mailto:
> robert@wibidata.com)> wrote:
> >> > Hey Everybody,
> >> >
> >> > I'd like to start a discussion about using automated code review tools
> >> > to improve the crunch development process. I'd I'm personally a big
> >> > fan of using tools that can help improve code quality. These tools
> >> > often include things like the maven-checkstyle-plugin and the maven-
> >> > findbugs-plugin. I am currently unaware of any comparable scala
> >> > automated tools. If people have opinions on whether or not we should
> >> > use tools like this or which tools we should be using specifically
> >> > please let us know.
> >> >
> >> > Also, another related question: Should we have some sort of a code
> >> > review process?
> >> >
> >> > Robert Chu
> >>
> >>
> >>
> >> --
> >> Director of Data Science
> >> Cloudera
> >> Twitter: @josh_wills
> >
> >
>

Re: Checkstyle & Reviews

Posted by Tom White <to...@cloudera.com>.
On Sat, Jun 16, 2012 at 3:27 AM, Gabriel Reid <ga...@gmail.com> wrote:
> +1 for a style guide and/or formatting rules and coding rules.
>
> For code style, something that I've seen work well (at least if everyone is using Eclipse) is to just have a shared code style definition that you can import into Eclipse, and then set Eclipse to apply that style whenever you save a file. This works great if everyone uses the same style definition, and breaks down rapidly if they don't.
>
> I don't have any experience using the maven-findbugs-plugin, but I do find the Checkstyle useful, as long as the settings are sane (which is not always the case).

We've had a good experience with Checkstyle in Apache Whirr. The way
we have it set up is to fail the package build if there are Checkstyle
errors. The relevant POM for doing that is at
http://svn.apache.org/repos/asf/whirr/trunk/pom.xml

Tom

>
> On Saturday 16 June 2012 at 00:17, Josh Wills wrote:
>
>> I'm in favor of a style guide; consistent layout makes it easier for
>> my brain to devour code. That said, I'm not up-to-date on best
>> practices here in the real world; my only experience with automated
>> code review tools was at teh goog.
>>
>> Re: code reviews, let's start that discussion up in another thread.
>>
>> J
>>
>> On Fri, Jun 15, 2012 at 3:13 PM, Robert Chu <robert@wibidata.com (mailto:robert@wibidata.com)> wrote:
>> > Hey Everybody,
>> >
>> > I'd like to start a discussion about using automated code review tools
>> > to improve the crunch development process. I'd I'm personally a big
>> > fan of using tools that can help improve code quality. These tools
>> > often include things like the maven-checkstyle-plugin and the maven-
>> > findbugs-plugin. I am currently unaware of any comparable scala
>> > automated tools. If people have opinions on whether or not we should
>> > use tools like this or which tools we should be using specifically
>> > please let us know.
>> >
>> > Also, another related question: Should we have some sort of a code
>> > review process?
>> >
>> > Robert Chu
>>
>>
>>
>> --
>> Director of Data Science
>> Cloudera
>> Twitter: @josh_wills
>
>

Re: Checkstyle & Reviews

Posted by Gabriel Reid <ga...@gmail.com>.
+1 for a style guide and/or formatting rules and coding rules.

For code style, something that I've seen work well (at least if everyone is using Eclipse) is to just have a shared code style definition that you can import into Eclipse, and then set Eclipse to apply that style whenever you save a file. This works great if everyone uses the same style definition, and breaks down rapidly if they don't.

I don't have any experience using the maven-findbugs-plugin, but I do find the Checkstyle useful, as long as the settings are sane (which is not always the case).

On Saturday 16 June 2012 at 00:17, Josh Wills wrote:

> I'm in favor of a style guide; consistent layout makes it easier for
> my brain to devour code. That said, I'm not up-to-date on best
> practices here in the real world; my only experience with automated
> code review tools was at teh goog.
> 
> Re: code reviews, let's start that discussion up in another thread.
> 
> J
> 
> On Fri, Jun 15, 2012 at 3:13 PM, Robert Chu <robert@wibidata.com (mailto:robert@wibidata.com)> wrote:
> > Hey Everybody,
> > 
> > I'd like to start a discussion about using automated code review tools
> > to improve the crunch development process. I'd I'm personally a big
> > fan of using tools that can help improve code quality. These tools
> > often include things like the maven-checkstyle-plugin and the maven-
> > findbugs-plugin. I am currently unaware of any comparable scala
> > automated tools. If people have opinions on whether or not we should
> > use tools like this or which tools we should be using specifically
> > please let us know.
> > 
> > Also, another related question: Should we have some sort of a code
> > review process?
> > 
> > Robert Chu
> 
> 
> 
> -- 
> Director of Data Science
> Cloudera
> Twitter: @josh_wills