search

mcupak / beacon-of-beacons

Beacon of Beacons.
http://mcupak.github.io/beacon-of-beacons
14 stars 10 forks source link

Set up checkstyle #69

Open mcupak opened 9 years ago

mcupak commented 9 years ago

Set up Checkstyle for the backend code using Maven Checkstyle Plugin.

rishabhsethi commented 9 years ago

I have created a pull request for this issue: https://github.com/mcupak/beacon-of-beacons/pull/71 Please check it. Thanks

mcupak commented 9 years ago

@rishabhsethi Thanks!

A few notes though:

rishabhsethi commented 9 years ago

Thanks for the review. I will sort this out soon. Can you please clarify what should I check in the customized checkstyle?

mcupak commented 9 years ago

The two things I mentioned (plus whatever you consider appropriate, if there is anything else). I.e. line width and usage of magic numbers.

rishabhsethi commented 9 years ago

I have made the suggested changes. Although I am still not sure what more can be ignored from the checkstyle rules (I am sure many useless checks are still there). Will make changes as I become more familiar with the code base.

mcupak commented 9 years ago

I played around with the style a bit and I'd suggest going with Google's code style, which I'll update a bit once you submit a PR.

The checkstyle config is here: https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml

The thing I don't like in your PR is that it uses a different config file for each module - that's redundant and prone to introduce inconsistencies. Please use a single configuration file. Here's a guide describing how to use checkstyle in a multi-module Maven project: https://maven.apache.org/plugins/maven-checkstyle-plugin/examples/multi-module-config.html

Can you update the PR?

Thanks!