Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set up checkstyle #69

Open
mcupak opened this issue Mar 19, 2015 · 6 comments
Open

Set up checkstyle #69

mcupak opened this issue Mar 19, 2015 · 6 comments

Comments

@mcupak
Copy link
Owner

mcupak commented Mar 19, 2015

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

@rishabhsethi
Copy link

I have created a pull request for this issue:
#71
Please check it.
Thanks

@mcupak
Copy link
Owner Author

mcupak commented Mar 23, 2015

@rishabhsethi Thanks!

A few notes though:

  • Why is it set up for two of the modules only? It should work for bob-data and bob-service as well, and should be configured in the parent POM.
  • Note that the parent POM fixes versions of all the components, please specify the version of the checkstyle plugin there as well.
  • I think the default set of rules is too strict - can you configure it not to report stuff like 80-character line width and magic numbers in hashCode methods?

@rishabhsethi
Copy link

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

@mcupak
Copy link
Owner Author

mcupak commented Mar 23, 2015

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
Copy link

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
Copy link
Owner Author

mcupak commented Mar 25, 2015

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants