-
Notifications
You must be signed in to change notification settings - Fork 482
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
Adding Detekt in the project #216
Conversation
Is detekt being applied to all modules? |
The plugin is applied only to the app module, but the configuration is set to check files across the entire project (all the modules). I wrote a pre-compiled plugin with the detekt configuration. private val projectSource = file("$rootDir")
private val configFile = files("$rootDir/config/detekt/detekt.yml")
private val baselineFile = file("$rootDir/config/detekt/baseline.xml")
private val kotlinFiles = "**/*.kt"
private val resourceFiles = "**/resources/**"
private val buildFiles = "**/build/**"
private val generatedFiles = "**/generated/**"
private val scriptsFiles = "**/*.kts" Basically, the tool will start in the include(kotlinFiles)
exclude(resourceFiles, buildFiles, generatedFiles, scriptsFiles) |
Can you like make a convention plugin and apply it to per module? Don't really want to run the whole task with |
I just realized that the pre-compiled plugin that I wrote is, in fact, a |
I plan to migrate everything to convention plugin |
@AntsyLich Let me know what you think. |
@AntsyLich Please ping me before merging this PR. |
I may have forgotten to ask you to rebase. Do that and ping me I'll merge it afterwards |
@AntsyLich Done. |
We don't need that huge ass yml file do we? Also use https://github.com/mrmans0n/compose-rules (https://mrmans0n.github.io/compose-rules/detekt) |
@AntsyLich |
it's fine to slim down the config file. chances are we will just keep following the default |
@AntsyLich Done. |
According to https://mrmans0n.github.io/compose-rules/detekt/ you need to enable some rules 🤔 one of them also seem to supersede |
@AntsyLich The plugin rules are enabled by default after applying the plugin. I trimmed down the config files removing all the rules with default values (as you asked). Here is the generated config file after applying the plugin. You can also see that the baseline has several compose-rules on it. |
Any reason why you didn't use detekt v1.23.4? |
* Removing ktlint * Removing compose lint * Adding initial Detekt config * Setting up detekt config * Adding detekt baseline * Fixing workflows * Moving to a module based solution * Adding new line * Adding new line * Updating baseline * Addressing PR suggestions * Regenerating baseline.xml * Cleanup --------- Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com> (cherry picked from commit cc09230) # Conflicts: # .github/workflows/build_pull_request.yml # .github/workflows/build_push.yml
Description:
This pull request aims to enhance the quality of our source code by replacing the ktlint code formatter with Detekt. Detekt is a powerful static analysis tool for Kotlin, providing comprehensive analyses to improve code readability and maintainability. Here are some of the benefits of using this tool:
Comprehensive Static Analysis:
Flexible Configuration:
Integration with Android Studio:
Baseline for Existing Errors:
Other changes
Workflow Updates:
Future work
In a subsequent PR, the baseline will be removed, and existing errors will be addressed. This phased approach allows us to adopt Detekt gradually without overwhelming the team with immediate corrections.
How to Test:
./gradlew detekt
to ensure Detekt's static analyses are executed successfully.More info:
Here is a report for all the issues found by the tool.
report.zip
Please let me know what you guys think.