Hi, It is great to have a code standard, it is even better when it is maintained, but when checkstyle reports error after several minutes building the code, while the code should be ok and it reports an issue as important as a trailing white space, then I believe anyone would more likely call it a pain in the A than a useful tool. What about a non-enforcing configuration, or activation only in a profile? And then jenkins could send the usual error messages when it breaks, and we can fix it later.
On Mon, Nov 4, 2013 at 4:33 PM, Hugo Trippaers <h...@trippaers.nl> wrote: > Hey John, > > That would be my idea. > > Make it mandatory for new (maven) projects coming into the code base and > slowly start working on fixing the existing projects. The current > checkstyle setting is very relaxed, just a few basic checks. Stuff that we > could technically fix with a few well written regular expressions even. > Over time we can start implementing parts of our code style in the > checkstyle config, but that is long term planning. > > Cheers, > > Hugo > > On 4 nov. 2013, at 16:28, John Kinsella <j...@stratosec.co> wrote: > > > I think it'd be fairly painful to make it mandatory - maybe see if we > can set that as a goal for 6 months out? > > > > On Nov 4, 2013, at 6:29 AM, Hugo Trippaers <h...@trippaers.nl<mailto: > h...@trippaers.nl>> > > wrote: > > > > Hey, > > > > Just added a very basic checkstyle configuration to maven. The > configuration file is in parents/checkstyle and it checks just a few very > basic things, like trailing whitespace and tabs where there should be > spaces. > > > > I’ve enabled it for a single plugin to just the impact on build time and > the amount of generated errors. Quite considerable, but i hope other parts > of the code are better ;-) > > > > You can enable check style for your plugin by adding the following to > your build plugins config in maven: > > > > <plugin> > > <groupId>org.apache.maven.plugins</groupId> > > <artifactId>maven-checkstyle-plugin</artifactId> > > <version>${cs.checkstyle.version}</version> > > <dependencies> > > <dependency> > > <groupId>org.apache.cloudstack</groupId> > > <artifactId>checkstyle</artifactId> > > <version>0.0.1-SNAPSHOT</version> > > </dependency> > > </dependencies> > > <executions> > > <execution> > > <phase>process-sources</phase> > > <goals> > > <goal>check</goal> > > </goals> > > </execution> > > </executions> > > <configuration> > > <failsOnError>true</failsOnError> > > <configLocation>tooling/checkstyle.xml</configLocation> > > <consoleOutput>true</consoleOutput> > > <includeTestSourceDirectory>true</includeTestSourceDirectory> > > <sourceDirectory>${project.basedir}</sourceDirectory> > > > <includes>**\/*.java,**\/*.xml,**\/*.ini,**\/*.sh,**\/*.bat</includes> > > <excludes>**\/target\/,**\/bin\/</excludes> > > </configuration> > > </plugin> > > > > > > For now its voluntary, but i would like your opinion on making this a > mandatory part of the build process. Meaning a compile with not succeed > when check style reports errors. > > > > Cheers, > > > > Hugo > > > > Stratosec<http://stratosec.co/> - Compliance as a Service > > o: 415.315.9385 > > @johnlkinsella<http://twitter.com/johnlkinsella> > > > > -- EOF