add checkstyle (no tabs, left curly) and usage docs #5075

[pdurbin]

connects to #5075

[pameyer]

Code says "sun conventions"; docs say "default Netbeans formatting settings". Are these different, and if so which one is authoritative?

[pdurbin]

Hmm. Good point. Maybe I'll take this whole section out. It's confusing. Thanks.

[pdurbin]

Removed in 80334fc

[pameyer]

If possible, html checkstyle reports might be more readable than xml reports.

[pdurbin]

It's a great idea to have HTML reports from Checkstyle but from a quick look at https://stackoverflow.com/questions/6342211/how-to-generate-checkstyle-reports we might be able to use mvn site to create the HTML for. I played with mvn site 6 or so years ago at https://github.com/dvn/dvn-mavensitepoc

[pameyer]

Does checkstyle support reformatting (either individual files, or ideally "only reformat code you've changed")?

[pdurbin]

I don't know.

[poikilotherm]

Nope. It will only validate things, not fix it for you.

[pdurbin]

If we switch to Go, we can use gofmt, which will reformat as well. 😄 See https://groups.google.com/d/msg/dataverse-dev/y2Jpk3szTf8/rckKmP6-BgAJ

[pameyer]

Is the convention for checkstyle.xml to live at the top-level, or should it live somewhere else (conf/? )?

[poikilotherm]

That depends on your personal opinion. From the Java projects I know, people just place it at the same level as the pom.xml.

[pdurbin]

I like it when I find certain files at the top level, including:

• README.md
• LICENSE.md
• CONTRIBUTING.md
• Dockerfile
• Vagrantfle

and for Java projects, seeing checkstyle.xml at the top level seems nice. 😄

[qqmyers]

I cherry-picked this commit and was able to run the check - only ~200 errors! Never-the-less, I agree this is a good place to start and this is a good add.

FWIW, the three issues I'm seeing where my code differs from what's there (which has its own variations) are

• tabs instead of spaces, and it looks like the standard indent is 4 spaces (not 2 as it said in the docs somewhere)
• Line length - mostly it appears that lines can be as long as necessary although there are some that have been split
• brace placement, though this is less prevalent.

I use eclipse rather than netbeans, but, after learning from @pdurbin that spaces were preferred, I was able to find how to change to four spaces. If it is possible to get a standard for how long a line can be before it wraps, I can probably match netbeans there too. I'll look into curly braces - the new doc shows how they should be and that may be the eclipse default already.

I guess one point in that is that giving specifics in the docs beyond just 'as netbeans does it' is helpful.

In terms of cleaning things up, it may be worth a run through the code (happy to help) but having a one-way policy for new code, i.e. if you submit ill-formatted code, others can reformat it without asking (or you may be required to fix the PR) and, if you find ill-formatted code, you're allowed to add fixes to your PR, could be helpful too. If things like the tab issues weren't so widespread now, I might suggest just starting with a one-way policy, but it would definitely make the first few PRs hard to read.

[pdurbin]

@qqmyers thanks for all of your thoughts on this. In 6ca8617 I specified 4 spaces for Java. For Bash I said 2 spaces in pull request #5063.

[michbarsinai]

Should we enable these? I think we're in pretty good shape in this area, but it's good to have this enforces, especially when people with non-Java (=no camelCase) background contribute.