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

Bump ktlint to version 0.46.1 and implement new rules #5040

Closed
wants to merge 9 commits into from

Conversation

kvn-stgl
Copy link
Contributor

@kvn-stgl kvn-stgl commented Jul 6, 2022

See https://github.com/pinterest/ktlint/releases/tag/0.46.0 and https://github.com/pinterest/ktlint/releases/tag/0.46.1 for full release notes.

What did I do:

@kvn-stgl
Copy link
Contributor Author

kvn-stgl commented Jul 6, 2022

I'm unsure how I should proceed with the filename rule.
Problem:

A file containing only one (non private) top level declaration (class, interface, object, type alias or function) must be named after that declaration. The name also must comply with the Pascal Case convention. The same applies to a file containing one single top level class declaration and one ore more extension functions for that class. filename (pinterest/ktlint#1117)
So we have a lot of file naming changes that lead to API changes. Currently, the apiCheck task fails because the changes are not binary compatible. So should I supress the filename warnings or should dump a new API file?

@BraisGabin
Copy link
Member

BraisGabin commented Jul 6, 2022

Would it be possible to split this PR? It's big to make a proper review.

I'm thinking in somethin like this:

  • One updating ktlint and enable by default no new non-experimental rules (but enabling them since 1.22.0 instead of `1.21.0).
  • Other with the different fixes/refactors
  • And other more adding the new rules.

I don't know how possible is what I'm asking for but it would help a lot.


I'm unsure how I should proceed with the filename rule.

I would disable that rule. We had similar rule on detekt that it's already enabled by default: MatchingDeclarationName.

@kvn-stgl
Copy link
Contributor Author

kvn-stgl commented Jul 7, 2022

Alright. I created a new PR to track the code changes a bit better. The PR is #5044

@kvn-stgl kvn-stgl closed this Jul 7, 2022
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.

None yet

2 participants