Skip to content
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

Add wrappers for new rules in ktlint 0.48 #5627

Closed
3flex opened this issue Dec 17, 2022 · 8 comments · Fixed by #6037
Closed

Add wrappers for new rules in ktlint 0.48 #5627

3flex opened this issue Dec 17, 2022 · 8 comments · Fixed by #6037
Labels
formatting never gets stale Mark Issues/PRs that should not be closed as they won't get stale rules

Comments

@3flex
Copy link
Member

3flex commented Dec 17, 2022

Expected Behavior of the rule

I can enable the following ktlint rules:

  • Class naming
  • Function naming
  • Property naming

Context

We have wrappers for other ktlint rules. We should add wrappers for the rules added in ktlint 0.48.0 (see #5625).

@3flex 3flex added the rules label Dec 17, 2022
@cortinico
Copy link
Member

There are three new rules that I haven't added wrappers for, since we seem to have the same ones in detekt already:

RE To this: I believe we should add those wrappers. The reason is that we've been doing that in the past to keep feature parity between bare Ktlint and our wrapper. We do already have other rules between first-party & our ktlint wrapper that have overlapping features. I won't diverge now

@github-actions
Copy link

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Mar 20, 2023
@3flex 3flex added never gets stale Mark Issues/PRs that should not be closed as they won't get stale and removed stale labels Mar 20, 2023
@atulgpt
Copy link
Contributor

atulgpt commented Apr 21, 2023

Hi @cortinico / @3flex so do we want to add the wrapper or not for these new rules?

@cortinico
Copy link
Member

As mentioned above, I think we should add wrapper for all the rules

@3flex
Copy link
Member Author

3flex commented Apr 23, 2023

It's up for grabs @atulgpt if you want to work on it?

@atulgpt
Copy link
Contributor

atulgpt commented Apr 23, 2023

Sure. Will this conflict with #6028? Or should I create a PR against your branch?

@3flex
Copy link
Member Author

3flex commented Apr 23, 2023

Yes, it will conflict. I've just pushed the last changes for that PR and don't expect to make any more, so it should be safe to work off that branch until it's merged.

@atulgpt
Copy link
Contributor

atulgpt commented Apr 23, 2023

Hi, @3flex I have created a 3flex#17 #6037 based from your PR(#6028).

Although I have a few queries, 1. We already have ClassNaming(at https://github.com/detekt/detekt/blob/main/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/ClassNaming.kt) should we support both rules?
2. While running ./gradlew generateDocumentation it changed default-detekt-config.yml the at https://github.com/3flex/detekt/pull/17/files#diff-f4f2976a0cb59a2678eb657bcd337cd6cb115f5543f02a22ca20b928701f5629R312 is that correct behaviour?

cc @cortinico

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatting never gets stale Mark Issues/PRs that should not be closed as they won't get stale rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants