You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@servicemix.apache.org by Bruce Snyder <br...@gmail.com> on 2007/07/25 18:07:01 UTC

Checkstyle and PMD

1) Is there a reason why the maven-checkstyle-plugin is enabled in
many subproject level POMs instead of just in the build module POM? It
seems like this should be enabled in one location instead of being
spread throughout 14 POMs across the code base. Even the PMD ruleset
is only enabled in build module POM.

2) It is a *major* pain to have a build fail simply because the code
is not formatted according to the Checkstyle and PMD conventions being
used in ServiceMix. So I've taken a stab at creating a code formatter
profile for Eclipse and some quick docs located here:

http://cwiki.apache.org/confluence/display/SM/Building#Building-CodeConventions

If you're using Eclipse, please try it out and improve it if necessary.

Bruce
-- 
perl -e 'print unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
);'

Apache Geronimo - http://geronimo.apache.org/
Apache ActiveMQ - http://activemq.org/
Apache ServiceMix - http://servicemix.org/
Castor - http://castor.org/

Re: Checkstyle and PMD

Posted by Bruce Snyder <br...@gmail.com>.
On 7/26/07, Daniel Kulp <dk...@apache.org> wrote:
>
> Yep.  When you specifically activate a profile via the command line, all
> the "activeByDefault" profiles are deactivated.   Thus, if you have the
> checks in an activeByDefault profile, they will run when you just
> type "mvn", but when you run -Pnochecks, the nochecks profile activates
> instead of the "checks" profile and the checks don't run.   In this
> case, the "nochecks" profile is empty in that it doesn't configure
> anything.

OK, so that makes sense to me. I had no idea that there was a checks
profile - now I know.

> CXF does it that way and it works really well.   The "hard core"
> developers know about -Pnochecks and -Pfastinstall (fastinstall also
> skips the tests) and use them.   However, the "getting starting" people
> run mvn which executes the checks.   This makes sure patches that they
> submit are formatted correctly and such which makes applying the patches
> easier.
>
> For CXF, we also have a -Psetup.eclipse profile that REALLY gets things
> setup in eclipse. (goes way beyond eclipse:eclipse)   It injects the CXF
> checkstyle rules into the workspace, sets the workspace warning levels
> appropriately, injects the code format template, import order, etc....
> For each project, it also sets up Checkstyle and PMD (assumes you have
> the checkstyle and PMD plugins installed).     Thus eclipse does all the
> code format checks for me.    Thus, I usually run with -Pnochecks.

I've seen these goals in the CXF POMs before and just figured that
they were CXF-specfiic. So are fastinstall and setup.eclipse
specifically for CXF or are they profiles in Maven? I like the idea of
the setup.eclipse profile because I just created an Eclipse code
convention profile and I was looking at using the Checkstyle and PMD
Eclipse plugins as well.

Bruce
-- 
perl -e 'print unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
);'

Apache Geronimo - http://geronimo.apache.org/
Apache ActiveMQ - http://activemq.org/
Apache ServiceMix - http://servicemix.org/
Castor - http://castor.org/

Re: Checkstyle and PMD

Posted by Daniel Kulp <dk...@apache.org>.
Yep.  When you specifically activate a profile via the command line, all 
the "activeByDefault" profiles are deactivated.   Thus, if you have the 
checks in an activeByDefault profile, they will run when you just 
type "mvn", but when you run -Pnochecks, the nochecks profile activates 
instead of the "checks" profile and the checks don't run.   In this 
case, the "nochecks" profile is empty in that it doesn't configure 
anything.

CXF does it that way and it works really well.   The "hard core" 
developers know about -Pnochecks and -Pfastinstall (fastinstall also 
skips the tests) and use them.   However, the "getting starting" people 
run mvn which executes the checks.   This makes sure patches that they 
submit are formatted correctly and such which makes applying the patches 
easier.

For CXF, we also have a -Psetup.eclipse profile that REALLY gets things 
setup in eclipse. (goes way beyond eclipse:eclipse)   It injects the CXF 
checkstyle rules into the workspace, sets the workspace warning levels 
appropriately, injects the code format template, import order, etc....   
For each project, it also sets up Checkstyle and PMD (assumes you have 
the checkstyle and PMD plugins installed).     Thus eclipse does all the 
code format checks for me.    Thus, I usually run with -Pnochecks.

Dan


On Thursday 26 July 2007 04:29, Guillaume Nodet wrote:
> I'm sure Dan Kulp can comment on that but it seems that the profile
> which contains the checks is disabled when the empty one is activated.
>
> On 7/26/07, Bruce Snyder <br...@gmail.com> wrote:
> > On 7/26/07, Guillaume Nodet <gn...@gmail.com> wrote:
> > > As the profile is empty it simply means that the checks wont be
> > > configured and activated :-)
> >
> > How does an empty profile prevent the invocation of Checkstyle and
> > PMD? Sounds like Maven voodoo.
> >
> > Bruce
> > --
> > perl -e 'print
> > unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
> > );'
> >
> > Apache Geronimo - http://geronimo.apache.org/
> > Apache ActiveMQ - http://activemq.org/
> > Apache ServiceMix - http://servicemix.org/
> > Castor - http://castor.org/

-- 
J. Daniel Kulp
Principal Engineer
IONA
P: 781-902-8727    C: 508-380-7194
daniel.kulp@iona.com
http://www.dankulp.com/blog

Re: Checkstyle and PMD

Posted by Guillaume Nodet <gn...@gmail.com>.
I'm sure Dan Kulp can comment on that but it seems that the profile
which contains the checks is disabled when the empty one is activated.

On 7/26/07, Bruce Snyder <br...@gmail.com> wrote:
> On 7/26/07, Guillaume Nodet <gn...@gmail.com> wrote:
> > As the profile is empty it simply means that the checks wont be
> > configured and activated :-)
>
> How does an empty profile prevent the invocation of Checkstyle and
> PMD? Sounds like Maven voodoo.
>
> Bruce
> --
> perl -e 'print
> unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
> );'
>
> Apache Geronimo - http://geronimo.apache.org/
> Apache ActiveMQ - http://activemq.org/
> Apache ServiceMix - http://servicemix.org/
> Castor - http://castor.org/
>


-- 
Cheers,
Guillaume Nodet
------------------------
Principal Engineer, IONA
Blog: http://gnodet.blogspot.com/

Re: Checkstyle and PMD

Posted by Bruce Snyder <br...@gmail.com>.
On 7/26/07, Guillaume Nodet <gn...@gmail.com> wrote:
> As the profile is empty it simply means that the checks wont be
> configured and activated :-)

How does an empty profile prevent the invocation of Checkstyle and
PMD? Sounds like Maven voodoo.

Bruce
-- 
perl -e 'print unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
);'

Apache Geronimo - http://geronimo.apache.org/
Apache ActiveMQ - http://activemq.org/
Apache ServiceMix - http://servicemix.org/
Castor - http://castor.org/

Re: Checkstyle and PMD

Posted by Guillaume Nodet <gn...@gmail.com>.
As the profile is empty it simply means that the checks wont be
configured and activated :-)

On 7/26/07, Bruce Snyder <br...@gmail.com> wrote:
> On 7/26/07, Guillaume Nodet <gn...@gmail.com> wrote:
> > Note that you can disable the checks by adding -Pnochecks on the
> > command line iirc.
>
> Yeah I've seen that profile, but I just figured that it hadn't been
> fleshed out yet . Why is that profile empty?
>
> Bruce
> --
> perl -e 'print
> unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
> );'
>
> Apache Geronimo - http://geronimo.apache.org/
> Apache ActiveMQ - http://activemq.org/
> Apache ServiceMix - http://servicemix.org/
> Castor - http://castor.org/
>


-- 
Cheers,
Guillaume Nodet
------------------------
Principal Engineer, IONA
Blog: http://gnodet.blogspot.com/

Re: Checkstyle and PMD

Posted by Bruce Snyder <br...@gmail.com>.
On 7/26/07, Guillaume Nodet <gn...@gmail.com> wrote:
> Note that you can disable the checks by adding -Pnochecks on the
> command line iirc.

Yeah I've seen that profile, but I just figured that it hadn't been
fleshed out yet . Why is that profile empty?

Bruce
-- 
perl -e 'print unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
);'

Apache Geronimo - http://geronimo.apache.org/
Apache ActiveMQ - http://activemq.org/
Apache ServiceMix - http://servicemix.org/
Castor - http://castor.org/

Re: Checkstyle and PMD

Posted by Guillaume Nodet <gn...@gmail.com>.
Note that you can disable the checks by adding -Pnochecks on the
command line iirc.

On 7/26/07, Kit Plummer <ki...@gmail.com> wrote:
> On 7/25/07, Bruce Snyder <br...@gmail.com> wrote:
> >
> > On 7/25/07, Kit Plummer <ki...@gmail.com> wrote:
> >
> > > As long as I can "comment out" the feature at one place I don't mind the
> > > styleguide stuff.  But, you are right...seems like it would be more of a
> > > pre-commit hook thing.
> >
> > Well the problem is that Subversion commits don't go through the Maven
> > build; they are a completely separate process. What we might consider
> > doing is providing one install goal for development, one install goal
> > to be used prior to doing a commit and the deploy goal:
> >
> > 1) development install goal - used to develop, debug and experiment
> > with the Subversion code
> > 2) pre-commit install goal - used immediately prior to performing a
> > Subversion commit to ensure that code conventions are followed
> > 3) deploy goal - handles standard deployment to Maven repos and source
> > packaging
> >
> > What do you guys think?
> >
> > Bruce
> > --
> > perl -e 'print
> > unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
> > );'
> >
> > Apache Geronimo - http://geronimo.apache.org/
> > Apache ActiveMQ - http://activemq.org/
> > Apache ServiceMix - http://servicemix.org/
> > Castor - http://castor.org/
> >
>
> Sounds reasonable.  Should accommodate things well enough...
>


-- 
Cheers,
Guillaume Nodet
------------------------
Principal Engineer, IONA
Blog: http://gnodet.blogspot.com/

Re: Checkstyle and PMD

Posted by Kit Plummer <ki...@gmail.com>.
On 7/25/07, Bruce Snyder <br...@gmail.com> wrote:
>
> On 7/25/07, Kit Plummer <ki...@gmail.com> wrote:
>
> > As long as I can "comment out" the feature at one place I don't mind the
> > styleguide stuff.  But, you are right...seems like it would be more of a
> > pre-commit hook thing.
>
> Well the problem is that Subversion commits don't go through the Maven
> build; they are a completely separate process. What we might consider
> doing is providing one install goal for development, one install goal
> to be used prior to doing a commit and the deploy goal:
>
> 1) development install goal - used to develop, debug and experiment
> with the Subversion code
> 2) pre-commit install goal - used immediately prior to performing a
> Subversion commit to ensure that code conventions are followed
> 3) deploy goal - handles standard deployment to Maven repos and source
> packaging
>
> What do you guys think?
>
> Bruce
> --
> perl -e 'print
> unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
> );'
>
> Apache Geronimo - http://geronimo.apache.org/
> Apache ActiveMQ - http://activemq.org/
> Apache ServiceMix - http://servicemix.org/
> Castor - http://castor.org/
>

Sounds reasonable.  Should accommodate things well enough...

Re: Checkstyle and PMD

Posted by Bruce Snyder <br...@gmail.com>.
On 7/25/07, Kit Plummer <ki...@gmail.com> wrote:

> As long as I can "comment out" the feature at one place I don't mind the
> styleguide stuff.  But, you are right...seems like it would be more of a
> pre-commit hook thing.

Well the problem is that Subversion commits don't go through the Maven
build; they are a completely separate process. What we might consider
doing is providing one install goal for development, one install goal
to be used prior to doing a commit and the deploy goal:

1) development install goal - used to develop, debug and experiment
with the Subversion code
2) pre-commit install goal - used immediately prior to performing a
Subversion commit to ensure that code conventions are followed
3) deploy goal - handles standard deployment to Maven repos and source
packaging

What do you guys think?

Bruce
-- 
perl -e 'print unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
);'

Apache Geronimo - http://geronimo.apache.org/
Apache ActiveMQ - http://activemq.org/
Apache ServiceMix - http://servicemix.org/
Castor - http://castor.org/

Re: Checkstyle and PMD

Posted by Kit Plummer <ki...@gmail.com>.
On 7/25/07, Bruce Snyder <br...@gmail.com> wrote:
>
> On 7/25/07, Guillaume Nodet <gn...@gmail.com> wrote:
> > If people find this configuration too painful, we can easily disable
> > the checks by default but i thought it was a good way to enforce a
> > coherent code format. And yes, all the configuration will be moved to
> > a single place when all the components pass the checks.
>
> Yeah, I was going to suggest this because they make experimentation
> very difficult. I also think that creating the source tarballs/zips
> should be part of deploy goal and not part of install goal. I agree
> that it's a good idea to enforce the code conventions when something
> is going to be committed, but not during every build cycle while I'm
> debugging and experimenting.
>
> Bruce
> --
> perl -e 'print
> unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
> );'
>
> Apache Geronimo - http://geronimo.apache.org/
> Apache ActiveMQ - http://activemq.org/
> Apache ServiceMix - http://servicemix.org/
> Castor - http://castor.org/
>

As long as I can "comment out" the feature at one place I don't mind the
styleguide stuff.  But, you are right...seems like it would be more of a
pre-commit hook thing.

-- 
Kit Plummer
Nobody-in-Charge @ Black:Hole:Logic
http://www.blackholelogic.com

Re: Checkstyle and PMD

Posted by Bruce Snyder <br...@gmail.com>.
On 7/25/07, Guillaume Nodet <gn...@gmail.com> wrote:
> If people find this configuration too painful, we can easily disable
> the checks by default but i thought it was a good way to enforce a
> coherent code format. And yes, all the configuration will be moved to
> a single place when all the components pass the checks.

Yeah, I was going to suggest this because they make experimentation
very difficult. I also think that creating the source tarballs/zips
should be part of deploy goal and not part of install goal. I agree
that it's a good idea to enforce the code conventions when something
is going to be committed, but not during every build cycle while I'm
debugging and experimenting.

Bruce
-- 
perl -e 'print unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
);'

Apache Geronimo - http://geronimo.apache.org/
Apache ActiveMQ - http://activemq.org/
Apache ServiceMix - http://servicemix.org/
Castor - http://castor.org/

Re: Checkstyle and PMD

Posted by Guillaume Nodet <gn...@gmail.com>.
If people find this configuration too painful, we can easily disable
the checks by default but i thought it was a good way to enforce a
coherent code format. And yes, all the configuration will be moved to
a single place when all the components pass the checks.

On 7/25/07, Bruce Snyder <br...@gmail.com> wrote:
> On 7/25/07, Thomas Termin <tt...@blue-elephant-systems.com> wrote:
> > Bruce Snyder wrote:
> > > 1) Is there a reason why the maven-checkstyle-plugin is enabled in
> > > many subproject level POMs instead of just in the build module POM? It
> > > seems like this should be enabled in one location instead of being
> > > spread throughout 14 POMs across the code base. Even the PMD ruleset
> > > is only enabled in build module POM.
> > I guess it is because not all components are refactored.
>
> OK, I see now - it's a work in progress.
>
> > See for example https://issues.apache.org/activemq/browse/SM-933
> >
> > On some day there should be all components refactored and the checkstyle
> > will be enabled in only one POM.
> >
> > > 2) It is a *major* pain to have a build fail simply because the code
> > > is not formatted according to the Checkstyle and PMD conventions being
> > > used in ServiceMix. So I've taken a stab at creating a code formatter
> > > profile for Eclipse and some quick docs located here:
> > This prevents a user to checkin stuff which is not in the correct style.
> > There were a hint from Guillaume some day.
>
> Yes, I understand, hence my creation of the Eclipse code formatter
> profile. The Eclipse Checkstyle plugin (http://eclipse-cs.sf.net/)
> would work as well.
>
> A few years back I used to use an automatic code formatter with Ant,
> but I can't remember the name of it now. It wasn't Jalopy but I
> suppose Jalopy would work just fine. The idea with Jalopy is that it
> automatically formats the code according to the coding conventions.
>
> BTW, there's a whole list of code verifiers here:
>
> http://javatoolbox.com/categories/code-analysers-standards-verifiers
>
> Some will auto-reformat the source and some won't. Just thinking...
>
> Bruce
> --
> perl -e 'print
> unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
> );'
>
> Apache Geronimo - http://geronimo.apache.org/
> Apache ActiveMQ - http://activemq.org/
> Apache ServiceMix - http://servicemix.org/
> Castor - http://castor.org/
>


-- 
Cheers,
Guillaume Nodet
------------------------
Principal Engineer, IONA
Blog: http://gnodet.blogspot.com/

Re: Checkstyle and PMD

Posted by Bruce Snyder <br...@gmail.com>.
On 7/25/07, Thomas Termin <tt...@blue-elephant-systems.com> wrote:
> Bruce Snyder wrote:
> > 1) Is there a reason why the maven-checkstyle-plugin is enabled in
> > many subproject level POMs instead of just in the build module POM? It
> > seems like this should be enabled in one location instead of being
> > spread throughout 14 POMs across the code base. Even the PMD ruleset
> > is only enabled in build module POM.
> I guess it is because not all components are refactored.

OK, I see now - it's a work in progress.

> See for example https://issues.apache.org/activemq/browse/SM-933
>
> On some day there should be all components refactored and the checkstyle
> will be enabled in only one POM.
>
> > 2) It is a *major* pain to have a build fail simply because the code
> > is not formatted according to the Checkstyle and PMD conventions being
> > used in ServiceMix. So I've taken a stab at creating a code formatter
> > profile for Eclipse and some quick docs located here:
> This prevents a user to checkin stuff which is not in the correct style.
> There were a hint from Guillaume some day.

Yes, I understand, hence my creation of the Eclipse code formatter
profile. The Eclipse Checkstyle plugin (http://eclipse-cs.sf.net/)
would work as well.

A few years back I used to use an automatic code formatter with Ant,
but I can't remember the name of it now. It wasn't Jalopy but I
suppose Jalopy would work just fine. The idea with Jalopy is that it
automatically formats the code according to the coding conventions.

BTW, there's a whole list of code verifiers here:

http://javatoolbox.com/categories/code-analysers-standards-verifiers

Some will auto-reformat the source and some won't. Just thinking...

Bruce
-- 
perl -e 'print unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E<D\!G;6%I;\"YC;VT*"
);'

Apache Geronimo - http://geronimo.apache.org/
Apache ActiveMQ - http://activemq.org/
Apache ServiceMix - http://servicemix.org/
Castor - http://castor.org/

Re: Checkstyle and PMD

Posted by Thomas Termin <tt...@blue-elephant-systems.com>.
Bruce Snyder wrote:
> 1) Is there a reason why the maven-checkstyle-plugin is enabled in
> many subproject level POMs instead of just in the build module POM? It
> seems like this should be enabled in one location instead of being
> spread throughout 14 POMs across the code base. Even the PMD ruleset
> is only enabled in build module POM.
I guess it is because not all components are refactored.

See for example https://issues.apache.org/activemq/browse/SM-933

On some day there should be all components refactored and the checkstyle
will be enabled in only one POM.

> 2) It is a *major* pain to have a build fail simply because the code
> is not formatted according to the Checkstyle and PMD conventions being
> used in ServiceMix. So I've taken a stab at creating a code formatter
> profile for Eclipse and some quick docs located here:
This prevents a user to checkin stuff which is not in the correct style.
There were a hint from Guillaume some day.

> 
> http://cwiki.apache.org/confluence/display/SM/Building#Building-CodeConventions
> 
> 
> If you're using Eclipse, please try it out and improve it if necessary.
> 

Cheers
Thomas

-- 
Thomas Termin
_______________________________
blue elephant systems GmbH
Wollgrasweg 49
D-70599 Stuttgart

Tel    :  (+49) 0711 - 45 10 17 676
Fax    :  (+49) 0711 - 45 10 17 573
WWW    :  http://www.blue-elephant-systems.com
Email  :  Thomas.Termin@blue-elephant-systems.com

blue elephant systems GmbH
Firmensitz      : Wollgrasweg 49, D-70599 Stuttgart
Registergericht : Amtsgericht Stuttgart, HRB 24106
Geschäftsführer : Holger Dietrich, Thomas Gentsch, Joachim Hoernle