Add a linter to the project #5198
Comments
|
If I understand correctly the linter would perform a check on pull requests, like is currently done for unit tests as seen in the example below? This project has such a thing configured: Ideally it should also be runnable locally, maybe even in IDEs. |
|
Yup, we can add the linting checks in the same workflow file. |
|
If possible, I would like to work on this issue. |
|
I am trying to add ktlint to the project, and I am running into a error |
|
Hello @shankarpriyank, I noticed that the issue assigned to you is now over a year old, and I was wondering about the current status of the associated PR. If, for any reason, you're unable to continue working on it, would it be possible for me to take over this PR? I'm keen to contribute and help move this forward 😊. Thanks |
|
Hey @neeldoshii, please feel free to work on this. |
|
Did an inspection on ktlint library mentioned by @shankarpriyank klint (Pintrest). I believe integrating ktlint with our GitHub Actions could significantly improve our development workflow, especially for Kotlin code. As of 8th March 2024, our codebase composition stands at 55% Java and 44% Kotlin. Since ktlint is designed specifically for Kotlin, it unfortunately does not support our Java codebase. To ensure both parts of our codebase are equally maintained and follow best practices, I propose we integrate both klint for Kotlin and Checkstyle for Java. In future when the code base is permanently migrated to kotlin codebase we can remove the Checkstyle. What's your opinion over this @nicolas-raoul @RitikaPahwa4444? |
|
Hi @neeldoshii, just a suggestion, it would be better if you could first introduce the library in the project and focus on elevating the developer experience. I assume would you would have to mark baseline files too cause we would most likely not want to change existing code. |
|
Thanks for your feedback @shankarpriyank,
My plan for now is to run ktlint only on recently edited changes (addition & deletion) of the files. So my plan is to add it in the Github Action which will check it & test code quality when the contributor will commit just like our Run test & Generate AP.
You are right this will lead to many refactoring because of lint errors. For this local ktlint check I plan to perform it via Git Pre-commit Hook which will make sure to perform gradle ktlintcheck & format only to staging commit files. For now I plan to introduce ktlint first and later on checkstyle or any good library which is better for java codebase based on the feedback from other contributors which is better. |
I think to do this on CI, would be tough.
Its a good idea to run format on recently staged file, but I am not really sure about adding a pre-commit hook, as the ktlintCheck will take quite some time to execute and I am not sure its worth it to make this trade-off. But anyways, please go ahead and feel free to open a pr |
|
Progress Update : @nicolas-raoul @shankarpriyank No of errors Count : 119 (which is way less as its a small refactor)
You are right about ktlint check!! We can plan to add Below is the results of |
|
Created issue: #5662 |
|
are you looking for someone to run ktlintCheck and go through and manually fix the formatting issues to complete this issue? |
|
@tristan81 Yes! If you are interested, please comment on #5662 so that I can assign it to you. You can start working on it already. Thanks a lot! :-) |
What is the user problem or growth opportunity you want to see solved?
If we start using a linter, maintaining the code style in the project will be easier. Also this will save some time for too, the maintainers wont have to look out for styling issues in pr's the contributors to will have a easy time following the code style of the project
How do you know that this problem exists today? Why is this important?
I made a few pr's and there was need to fix the styling issues in them, by using a linter we can cut down on the to and fro
Who will benefit from it?
The developers/contributors and even the maintainers
Anything else you would like to add?
There are quite a few options we can look at, but I would suggest using ktlint
The text was updated successfully, but these errors were encountered: