Collect: Fix style throughout the codebase and check style as part of the build process

[lognaturel]

The code base is currently not very consistent when it comes to code style. We should agree on a style guide to follow, fix style across the codebase and enable style checks as part of the build process.

We already have a good checkstyle file. It can be run with gradle checkstyle and eventually added to continuous integration. I also like to use the checkstyle IDEA plugin in Android Studio.

I would like to propose the following changes to the existing checkstyle file:

• Enforce standard MethodName
• Enforce standard ParameterName
• Enforce standard StaticVariableName
• Require curly braces as in Android contributor guide. I'd like to go further than them and require them even for one-line ifs but I don't feel strongly about it. NeedBraces
• Require the opening curly brace to be on the end of the line of the test as in Android contributor guide. LeftCurly
• Require the closing curly brace to be on the same line as the next part of a multi-block statement as in Android contributor guide. RightCurly

We should also decide whether or not to enforce the common Android convention to use an m prefix for non-public, non-static fields and an s for non-public, static fields. I know @nribeka and @dcbriccetti have strong feelings about this.

And finally, we need to agree on a strategy for bringing the codebase in line with the configuration file in a way that is incremental and verifiable. Here are some options:

• a single pull request with a different commit for each problem fixed.
• one pull request per file.
• one pull request per category of issue.
• ???

[pratikmjoshi]

@lognaturel I think one pull request per category of issue with a different commit for each file in which the issue is solved can be another option.

[grzesiek2010]

I'm in favour of one pull request per category of issue. Can't wait for results :)

[nribeka]

I have a strong feeling of not using the Hungarian notation moving forward. You guys can read the pros and cons in wikipedia.

I think we're still mising some stuff like from checkstyle config:

• OneTopLevelClass
• EqualsAvoidNull

We can use the Google's one as some guideline I think.

[lognaturel]

How about we start with a blank checkstyle file and incrementally add checks that we fix across the code base. I have submitted an example at #832. I'd be fine with either moving towards Google's checkstyle file or android-check's easy file.

[lognaturel]

I had a bit more time to think about this and had a change in heart. @nribeka is right, let's use the Google checkstyle file since their style guide is what we're currently pointing people to. I now propose we start out with the whole thing in there but with checks commented out. Contributors can pick ONE check to uncomment, review the errors, fix them committing changes for each file and then issue a pull request. In some cases, we may want to include ODK-specific checks and those can be grouped at the top. For example, I added some import checks.

832 has been updated accordingly. How does that sound? @grzesiek2010 @pratikmjoshi @yanokwa @shobhitagarwal1612 @nribeka

[yanokwa]

I had the same idea so I agree!

[shobhitagarwal1612]

Sounds great

[grzesiek2010]

Sounds good to me as well.

[shobhitagarwal1612]

Can we close this now?

[lognaturel]

Sounds good to me.