Conversation
| id("de.fayard.buildSrcVersions").version(Versions.de_fayard_buildsrcversions_gradle_plugin) | ||
|
|
||
| val PluginDependenciesSpec.detekt: PluginDependencySpec | ||
| inline get() = |
There was a problem hiding this comment.
First time I see the inline + get in the same usage. Nice!
There was a problem hiding this comment.
Haha thanks! Honestly, I just saw this in the previous line and did a quick search for usefulness. Turns out inline getters are quite handy, so I kept it.
sierisimo
left a comment
There was a problem hiding this comment.
Good initial approach for Detekt, probably we will do changes to configuration as the code grows.
Nice job!
| } | ||
|
|
||
| detekt { | ||
| input = files("$projectDir/app/src/main/java") |
There was a problem hiding this comment.
How about adding also test files?
Something like:
input = files( "src/main/java", "src/test/java", "src/androidTest/java", )
There was a problem hiding this comment.
In my experience, the test files cause more warnings for the way they are written. I know it sounds like an excuse but it happens for example on methods or names.
There was a problem hiding this comment.
I agree with @sierisimo. I have used static analysis tools with test code before and it really looks bad if your CI pipeline fails because of code smells in your test code.
There was a problem hiding this comment.
Yes it's harder to keep maintainability on tests, and while I haven't been in a situation where this has been a real problem, I understand your concerns. LGTM :)
maestromac
left a comment
There was a problem hiding this comment.
Thanks for this PR @xuhaibahmad ! LGTM!
What type of PR is this? (check all applicable)
Description
Add Detekt for static analysis.
Related Tickets & Documents
Resolves #45
Screenshots/Recordings (if there are UI changes)
[optional] What gif best describes this PR or how it makes you feel?