Fundamentals : Checkstylework#694
Conversation
|
@dhaval24, I mostly want to review the config for checkstyle and where it was added to the build script. Is the config included in this PR? |
|
@littleaj yep /config/checkstyle/checkstyle.xml is the file for the checkstyle config. And it is added to the build script in common-java.gradle file. |
| import org.gradle.nativeplatform.SharedLibraryBinarySpec | ||
| import org.gradle.nativeplatform.StaticLibraryBinarySpec | ||
| import org.gradle.nativeplatform.platform.NativePlatform | ||
| import org.gradle.nativeplatform.toolchain.VisualCpp |
There was a problem hiding this comment.
These are used below. Removing this breaks the script.
There was a problem hiding this comment.
ah okay didn't carefully looked at it after checkstyle ran
| <message key="ws.notPreceded" | ||
| value="GenericWhitespace ''{0}'' is not preceded with whitespace."/> | ||
| </module> | ||
| <module name="Indentation"> |
There was a problem hiding this comment.
IMO, I think all these values should be doubled.
There was a problem hiding this comment.
Ok, let's take Gustavo's suggestion too and whatever is fit we can adopt to that.
| /** | ||
| * Created by gupele on 7/27/2015. | ||
| */ | ||
| /** Created by gupele on 7/27/2015. */ |
There was a problem hiding this comment.
I don't like this style. Javadoc should always be 3 lines like before.
| <property name="allowByTailComment" value="true"/> | ||
| <property name="allowNonPrintableEscapes" value="true"/> | ||
| </module> | ||
| <module name="LineLength"> |
There was a problem hiding this comment.
I'd prefer to remove this module. A lot of the changes done are because of this rule and most of them make the code look worse, IMO.
|
@littleaj thanks let's use one triage meeting to finalize the rules and then it would be easy to apply! This was just to see how it works with build |
| <message key="name.invalidPattern" | ||
| value="Method name ''{0}'' must match pattern ''{1}''."/> | ||
| </module> | ||
| <module name="SingleLineJavadoc"> |
There was a problem hiding this comment.
(Mentioned in another file where this was applied) I'd like to remove this module.
|
@dhaval24 Sounds good. |
|
@littleaj Abandoning the PR. I will create a new one with only |
This PR is to enforce checkstyle rules on the entire project. The style guide is under /config/checkstyle/checkstyle.xml
I have currently used Google Java Style for this and integration in the build is done using the Gradle Checkstyle plugin.
This PR is an initial commit, and we can adopt to and relax some of the style rules here.
@littleaj @grlima please take a look and feel free to add commits on top to update the style as per our usecase here.