fix:Made some changes so that errors show up during ktlint pr review

[angrezichatterbox]

Contributor checklist

• This pull request is on a separate branch and not the main branch

Description

• The workflow has been changed to use tools from the GitHub marketplace. Currently, I have not set a config file for the ktlint check. However, at a later point, a config file could be added to tweak the lint checks.

Related issue

• Kotlin linting workflow #70

[github-actions]

Thank you for the pull request!

The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Android rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you!

Maintainer checklist

• The commit messages for the remote branch should be checked to make sure the contributor's email is set up correctly so that they receive credit for their contribution

  • The contributor's name and icon in remote commits should be the same as what appears in the PR
  • If there's a mismatch, the contributor needs to make sure that the email they use for GitHub matches what they have for git config user.email in their local Scribe-Android repo

• The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

[angrezichatterbox]

I have made some tests in my repo using some ktlint pr workflow tool from GitHub marketplace and found that this one works well and a config file could be added to tweak the lint checks.

[andrewtavis]

Awesome, @angrezichatterbox! Really appreciate your research here!

[andrewtavis]

Will bring this in tonight

[andrewtavis]

Hey @angrezichatterbox 👋 Sorry for the changes here. I'm looking at a couple of Kotlin starter projects, and it does seem like ktlint and detekt are normally ran without plugins. I was specifically checking krzdabrowski/android-starter-2022, but then the way that plugins are managed is different. Could you take another look at this and figure out the missing ktlint and detekt from the project? I think that this way's a bit more standard, from what I've been seeing.

Feedback would be appreciated! I just don't really see the need to add in reviewdog into this as well via the action you found when there are some solid options out there :)

[angrezichatterbox]

Hey @angrezichatterbox 👋 Sorry for the changes here. I'm looking at a couple of Kotlin starter projects, and it does seem like ktlint and detekt are normally ran without plugins. I was specifically checking krzdabrowski/android-starter-2022, but then the way that plugins are managed is different. Could you take another look at this and figure out the missing ktlint and detekt from the project? I think that this way's a bit more standard, from what I've been seeing.

Feedback would be appreciated! I just don't really see the need to add in reviewdog into this as well via the action you found when there are some solid options out there :)

I will take a look into adding those plugins to build.gradle.
The approach is much better. My approach was not the right as I was trying to use different tools in the GitHub marketplace for this. The gradle checks using those plugins are a solid approach and much more flexible in customizing from what I could see in the docs.

Also after this issue, I guess we could have an issue migrating from Groovy build files to Kotlin build configuration as I could see most docs just mentioning adding their plugin to kotlin build config files directly. It would be really helpful for this project moving forward.

[angrezichatterbox]

detekt is added to grade and the run seems to be fine. ktlint works fine but lintKotlin doesn't seem to be added as a task when ktlint plugin is added I will look into it and try figuring it out :).

[andrewtavis]

Thanks for all the efforts here, @angrezichatterbox! And yes I was also thinking that a next issue could be to migrate over to directly installed plugins :) Feel free to make that issue whenever, or I'm also happy to 😊

[angrezichatterbox]

Thanks for all the efforts here, @angrezichatterbox! And yes I was also thinking that a next issue could be to migrate over to directly installed plugins :) Feel free to make that issue whenever, or I'm also happy to 😊

I have added the lintKotlin to also work It would fail as there are linting issues but the command no longer shows its not present in the tasks. I will create an issue for migrating from groovy to Kotlin.

[andrewtavis]

This is great, @angrezichatterbox! Nice to see that it's working, regardless of the errors 😊 Do you want to look into a way of disabling them and we can make good first issues to undisable the issue and fix the linting errors, or should we merge and just know that future PRs will fail the linting check? We can make the issues to fix the errors either way :)

[angrezichatterbox]

This is great, @angrezichatterbox! Nice to see that it's working, regardless of the errors 😊 Do you want to look into a way of disabling them and we can make good first issues to undisable the issue and fix the linting errors, or should we merge and just know that future PRs will fail the linting check? We can make the issues to fix the errors either way :)

Yes we can disable some of them for now. I could use ./gradlew formatKotlin command for an linting issues that can be auto formatted and then I could see the linting errors left and then disable them.

[andrewtavis]

Is there a way to disable the rules one by one, so like disabling the indentation errors? No stress on this though. I'd be happy to make individual issues for this and the workflow can just fail until it doesn't :)

[angrezichatterbox]

Is there a way to disable the rules one by one, so like disabling the indentation errors? No stress on this though. I'd be happy to make individual issues for this and the workflow can just fail until it doesn't :)

Yes, It is possible to disable them one by one. I will be disabling them one by one.

[angrezichatterbox]

I have disabled the linting issues for now. There are 4 that I have disabled.

[andrewtavis]

Nice, @angrezichatterbox! Do you want to make four good first issues for how to enable the linting checks you disabled and give advice on how to solve them?

[andrewtavis]

Or let's do this over the weekend once this is merged :) Will do it later!

[angrezichatterbox]

I could make issues for those once this is merged. :)

[andrewtavis]

Thank you for the work in settings this up, @angrezichatterbox, and beyond that for taking the time to fix the initial errors! 😊 Is already looking so much better, and this is really going to improve the development experience for people :)