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

Integrate with a Kotlin linter #4

Closed
u-ways opened this issue Oct 1, 2022 · 9 comments · Fixed by #9
Closed

Integrate with a Kotlin linter #4

u-ways opened this issue Oct 1, 2022 · 9 comments · Fixed by #9
Assignees
Labels
cicd Continuous integration / continuous deployment related tasks enhancement New feature or request hacktoberfest A month-long celebration run by DigitalOcean to celebrate and give back to open source projects.

Comments

@u-ways
Copy link
Member

u-ways commented Oct 1, 2022

It would be nice to have a linter to keep the project as consistent as possible. A widely popular Kotlin linter is Pinterest's ktlint. It has a built-in rule sets, formatter tasks, reporters, and allows extension with custom rule sets and reporters.

Requirements

  • Set CI to run the linter after tests has passed. (must be a Makefile recipe)
  • Provide an .editorconfig to allow IDE users adhere to the standards for a smoother dev experience.

Please see the user guide for further details.

@u-ways u-ways added cicd Continuous integration / continuous deployment related tasks enhancement New feature or request hacktoberfest A month-long celebration run by DigitalOcean to celebrate and give back to open source projects. labels Oct 1, 2022
@doriancodes
Copy link
Contributor

I can take over this issue :)

@u-ways
Copy link
Member Author

u-ways commented Oct 2, 2022

@doriancodes Awesome, this has now been assigned to you.

@u-ways
Copy link
Member Author

u-ways commented Oct 2, 2022

@doriancodes just a heads up that recent similar work has been completed/accepted here: u-ways/kotlin-quarkus-realworld-example-app#8

@doriancodes
Copy link
Contributor

doriancodes commented Oct 2, 2022

I have some questions:

  • would you prefer to use a gradle plugin or the cli tool directly?
  • are you aware of this problem of adding .editoconfig file for Intellij users :

    Unfortunately IntelliJ IDEA has an autoformat issue regarding .editorconfig. Due to this error an additional space is added between glob statements, resulting in [*{kt, kts}] instead of [*{kt,kts}]. The .editorconfig library used by ktlint ignores sections after encountering a space in the list. As a result, the rule is not applied on all files as documented in the original ktlint issue.

  • do you want to run the ktlint command just to check if there are files that need formatting (so without autoformat feature)? If that's the case wouldn't make sense to run the ktlint command before the tests?

@doriancodes
Copy link
Contributor

update: I just saw https://github.com/u-ways/kotlin-quarkus-realworld-example-app/pull/8/files and answers most of my questions :)

@u-ways
Copy link
Member Author

u-ways commented Oct 2, 2022

@doriancodes glad to read it helped.

  • Yes, please use the Gradle plugin.
  • For the glob issue: Separating each glob seems to resolve the issue if I am not mistaken (i.e. [{\*.kt,\*.kts}] instead of [*{kt,kts}]). If that doesn't fix it, can you please advise how would you fix this? (I will look into this in my free time if it couldn't be resolved)
  • Finally, it would be preferable to run the Ktlinter after all tests has passed. This way, the dev will only have to modify their ktlint issues once at the end instead of having to modify again after fixing their tests.

@doriancodes
Copy link
Contributor

I found out this solution proposed https://pinterest.github.io/ktlint/rules/configuration-intellij-idea/ . It seems that there is a gradle task from the plugin to generate this configuration locally called ktlintApplyToIdea, do you want me to mention it in the README.md file?

@u-ways
Copy link
Member Author

u-ways commented Oct 2, 2022

I found out this solution proposed https://pinterest.github.io/ktlint/rules/configuration-intellij-idea/ . It seems that there is a gradle task from the plugin to generate this configuration locally called ktlintApplyToIdea, do you want me to mention it in the README.md file?

Awesome, yeah I have used this task before, and I recall there is a similar task that you can apply IDE-level too. I would suggest mentioning this in the CONTRIBUTING.md.

Since I am working on improving the documentation, I will add this in my todo list since you already submitted a PR, and it looks great.

@u-ways u-ways linked a pull request Oct 2, 2022 that will close this issue
@u-ways u-ways closed this as completed in #9 Oct 2, 2022
@u-ways
Copy link
Member Author

u-ways commented Oct 2, 2022

@doriancodes If you're interested in contributing more, you can have a look at other hacktoberfest tickets in the following repositories please:

Thanks again for your help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cicd Continuous integration / continuous deployment related tasks enhancement New feature or request hacktoberfest A month-long celebration run by DigitalOcean to celebrate and give back to open source projects.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants