-
-
Notifications
You must be signed in to change notification settings - Fork 756
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
How to make update the configuration easier #4517
Comments
Just dropping this here: #3002 (comment) IMHO this issue should be part of the broader discussion on how the configuration of detekt can be improved overall. Perhaps a config that allows for enabling all rules by default means the actual detekt config can be stripped back to either enable or disable app rules by default, then enabling or disabling individual rules as required. That should avoid requiring any wholesale changes to the config file with new detekt releases. |
Other point: Allow to instantiate the same rule multiple times. To me this is a huge change that have a lot of requirements. How can we work on this to make it happen? I worked already by my own in the DSL or the refactor of the |
We could also think about |
I personally don't think the config file format or DSL matters that much, though TOML might have properties that help with some of these goals (I haven't worked with it except for the Gradle versions catalog)
It's true, there are a lot of suggestions for improvement. My point wasn't so much that this is the right end state, only that we should start with a design and/or spec, that addresses the issues with the current config and the features we will target. I'm sure most of it can be added iteratively. I just want to avoid adding more config features without a plan in place, because we may make things harder for ourselves in future. |
My 2 cents: I think a web tool could help here. I would fork this tool we use for React Native https://github.com/react-native-community/upgrade-helper Essentially it allows you to diff two defined version of a library, to see what has changed. For example between We could adapt it to see how the default config file evolved (which at least for me is a bit of the pain point as the config file is rather big and hard to navigate sometimes). As an improvement, we could extend the web tool to apply the resulting |
This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days. |
How can we actionate this issue? Do we create a yaml differ and patcher (or find one)? Is it worth it? Are we going to continue working with yaml? |
From a dev perspective, updating detect rules is always a pain - it is hard to keep custom detekt config up to date (to have all rules in the config file). In ideal world I would like to see
Real-life scenario:
|
What about dropping configuration file types like Yaml and use Kotlin files/modules? I was thinking how to argument this but with this is enough: Type safety. Same problem GitHub Actions has and there is a Kotlin DSL with wrappers for common actions to generate the actions yaml files. Detekt wouldn't need to generate anything, just pick the list of rules provided via Kotlin with the kotlin type system. |
Have you considered using
In theory, I agree, it would be nice to add Kotlin support for config files (maybe support both)? |
It should be a JVM module added as included build or added to the classpath so you can pass it to the Detekt block. A JVM module is not too costly, I agree it is harder to create than just a yaml file but you get:
A lot of flexibility, unification, and safer way to work with Rules, but with a bit of overload (creating a module vs creating a file). |
side note that it comes will all the side effects of including dependencies that can clash with your Kotlin version + an included build increases your Gradle configuration time. |
I would go for something like *.detekt.kts. And an IDEA plugin to inject all the dependencies. The problem of a new jvm module is that you can have a different configuration per module so it doesn't scale. I would also like to have a dsl but I agree with @cortinico that it is a lot of tooling. But this topic is really important. Our configuration isn't easy and that's not good to onboard new people. |
@BraisGabin you can already add dependencies to The configuration itself should be the Gradle Detekt block, and the module/s itself should be a way to provide those configurations. For example, it is up to the team the way they manage the that/those modules, at the end, in the plugin block, you should be adding the rules, for example: Imagine that Detekt is setting detekt {
rules.set(defaultRules + publishedCompanyModuleRules + localProjectModuleRules)
} Similar behavior to Koin modules, if the same rule with a different value is provided two times, the latest has to preference and override the previous one. |
One really nice thing Detekt has, is being respectful with all or almost all Gradle best practices, which implies that projects are isolated, so it is the responsibility of a developer to provide the same rule sets to all modules in some way, probably via convention plugins as that is the recommended approach that the Gradle team is suggesting. But it is up to them how they manage Gradle, and if they want the same rules in all projects for Detekt or whatever other plugin. I think it is not a Detekt responsibility to force them to share them in a specific way, it is a Gradle (or the build tool) responsibility. Spotless is "similar" to Detekt and I think they haven't a problem with scaling and you have to provide the "rules" to each project |
Hi @BraisGabin I had a somewhat similar proposal at #6053 Even @cortinico suggestion at #6053 (comment) here valid and will work in streamlining the detekt defaults management |
To me the configuration of detekt is one of the major things that we should improve for 2.0 but I must say that I have no idea how to do it. Problems:
A possible solution is a DSL that generates a |
What if we add That way, whenever new rules are added, developer updating the plugin will get warned and can copy paste new rules from default config to the existing yml file. It is still a chore, but, assuming that there is fairly small amount of rules on each release, it sounds much easier than #3558 (comment) to me EDIT: this appears to be in already as |
Expected Behavior
When I update detekt my configuration of detekt is updated too to reflect the new defaults/rules/flags.
Current Behavior
Right now detekt doesn't do that and it's a pain to keep it updated.
Context
To be honest, I don't know how we can fix this issue. In my case I like to have ALL the configurations in my project so I can see all the flags of all the rules and configure it easily. I use the configuration like documentation or a documentation index. Using the script/algorithm linked in point 2 I can keep my documentation up to date but it's not as fast and easy as it should be. Problems:
sed
. But that's not replicable to other projects.yaml
s but I'm not using the fact that they areyaml
. So I get lots of conflicts that could be avoided if the merger would interpret the yaml instead of treting it as a plain file.Any ideas about how to make it easier? Should we include a
detektUpdate
task on our gradle plugin? How could we implement it?The text was updated successfully, but these errors were encountered: