Gradle, perfTest, Checkstyle

[mkalb]

Please review and test first, before merging this PullRequest.

Forum discussions:
http://forum.movingblocks.net/threads/terasology-code-style-conventions.501/
http://forum.movingblocks.net/threads/rename-perftest.529/

[x3ro]

These rules are great :) But as always, I have some suggestions for improvement:

IMHO every checkstyle rule should at least be briefly described inline, e.g.

<!-- Checks that long constants are defined with an upper ell. That is ' L' and not 'l'. -->
<module name="UpperEll"/>

That way you don't have to got back and forth between XML and their documentation, which makes the rules way easier to understand :)

Also I think that

<module name="NewlineAtEndOfFile" />

should be

<module name="NewlineAtEndOfFile">
    <!-- Check to make sure UNIX linefeeds are used -->
    <property name="lineSeparator" value="lf"/>
</module>

[mkalb]

Okay, I will add a short description to the rules.

The line separator should be handled by git. My local java files have windows line separators (\r\n).

[ghost]

Currently discussing inclusion of this pull request with Begla.

[Cervator]

Just tested this by pointing our Jenkins test job at @mkalb's branch - works! But it seems to aggressively flag everything protobuf and came up with more than 19 thousand violations just there :D

http://jenkins.movingblocks.net/job/TestCheckstyle/13/checkstyleResult/new/?

Maybe there's a way to exclude that from something? :-)

The new report modules seem clever though, was able to home in on some valid violations very easily, like whitespace quirks. I think this'll be a great help to make the code more consistent and a lot of the reports can be fixed easily by new contributors eager to find something easy to work on

Maybe we can sort the different modules a little by priority now that we have more of them? Whitespace should probably be low, missing brackets and stuff like that high.

[mkalb]

I added a suppression file with a suppression for protobuf.
I changed the severity for some checks from error to warning.

[Cervator]

Looking good! http://jenkins.movingblocks.net/job/TestCheckstyle/

Still going yellow from the unit tests and what not, but thinking about enabling it for Develop anyway, maybe we can encourage some beginning coders to help cut down on the stats. There are some cute "game" plugins for Jenkins to reward stuff like that. Just saw a new one that looks interesting and it works with Git. Sneaky project is in beta and you need to sign up 3 friends/team members to get in, anybody up for it? ;-)

http://buildcoin.com?lrRef=cm2Vn

The more usual one is over at https://wiki.jenkins-ci.org/display/JENKINS/The+Continuous+Integration+Game+plugin

[mkalb]

There are other useful check rules, but I think we should not start with more rules. The others can be added later.

The "suppression" has a problem. I cannot use it with eclipse because eclipse need an other file path. Mabye I add a eclipse_checkstyle.xml. Can someone test it with IntelliJ IDEA.

[Cervator]

That's with an actual IDE plugin (or built-in module) for checkstyle, right? I've just tried via command line and Jenkins, should be able to dig something up for IntelliJ

[mkalb]

Yes, Eclipse has an external checkstyle plugin. I commited a fix for this eclipse plugin.
Eclipse needs a full path, so I used a "hack" and defined the file as "${samedir}/suppressions.xml".
"samedir" is a eclipse property. And I also defined this property for gradle (build.gradle).
But I am not sure about IDEA. I will test it.

[Cervator]

Thanks :-)

[mkalb]

@Cervator I didn't want to close this pull request.

It works with IDEA :-)

1. Open Plugin-Manager
2. Browse repositories ...
3. Search for "ChechStyle"
4. Install "CheckStyle-IDEA
5. Open Project
6. File -> Settings...
7. Checkstyle
8. Add (select config/checkstyle/checkstyle.xml)
9. Edit properties: samedir = config/checkstyle

It works now with gradle command line, Jenkins, Eclipse and IDEA.
I am ready and it can be merged.

[Cervator]

Nice! Looks great. Have enabled for the primary job in Jenkins and we'll have to live with a yellow ball for a bit :D

Could define some thresholds probably, especially for checkstyle (rather than unit tests), but no rush. In the meantime let the fixing of easy things begin!

http://jenkins.movingblocks.net/job/Terasology/273/

[Cervator]

Incidentally, what's next? Findbugs? PMD? :D

Seems Gradle and Jenkins supports them both - http://docs.codehaus.org/display/GRADLE/Cookbook#Cookbook-usingFindbugs + https://wiki.jenkins-ci.org/display/JENKINS/PMD+Plugin etc

I suppose we could analyze too much, however :-)

[mkalb]

I think we should wait with PMD and FindBugs. There are more useful checkstyle rules which we can add first.

[Cervator]

Works for me, might spread out the love a bit, leave more easy tasks for later :-)

Then there's also Sonar to perhaps consolidate it all more later: http://www.sonarsource.org/