Skip to content

Incorporated Checkstyle, findbugs, pmd and lint#321

Merged
AndyScherzinger merged 1 commit intonextcloud:masterfrom
JubrilEdu:feature_code_quality_check
Oct 12, 2016
Merged

Incorporated Checkstyle, findbugs, pmd and lint#321
AndyScherzinger merged 1 commit intonextcloud:masterfrom
JubrilEdu:feature_code_quality_check

Conversation

@JubrilEdu
Copy link
Contributor

Incorporated checkstyle, findbugs, pmd and lint for code quality. Most rules are not set yet as that would be needing a lot of changes in the code base so the rules can be added bit by bit and corresponding changes effected in codebase. @AndyScherzinger

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Oct 9, 2016

Awesome work @djubreel 👍

@tobiasKaminsky please also have a look and if you are okay with it please merge. I did a dry run locally (gradlew.bat check) and I also set ignoreFailures = true in build.gradle so the build runs through and all checks get executed and reported (plus the build doesn't fail that way). Imho this is ready to merge. Then we should start new issues to resolve the reported issue (mainly findbugs and pmd so the build doesn't fail) and then activate this in our CI builds.

@tobiasKaminsky
Copy link
Member

Code is 👍
Can we merge it? Or do we have to change "ignoreFailures" to true before merging?

@tobiasKaminsky
Copy link
Member

One minor thing:

  • This should be mentioned in some readme/documentation

@AndyScherzinger
Copy link
Member

I would say let's wait with the "documentation" until the fails are fixes, then activate it in the builds and put it into the documentation.

We can merge right away since the checks will fail if you execute them, but they aren't activated in our builds just yet.

@AndyScherzinger AndyScherzinger merged commit 24bac65 into nextcloud:master Oct 12, 2016
@AndyScherzinger
Copy link
Member

@djubreel The PR has been merged! 🎉 Thank you very much for this contribution!!! 🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants