-
Notifications
You must be signed in to change notification settings - Fork 8
Migrate from code_climate to QLTY #282
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jreineckearm The basic results look promising, but we should have a discussion about the configuration settings, specifically
- Which checks/plugins should be enabled?
- What thresholds should be defined?
jreineckearm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Let's start with this set of settings. We may change though settings over time and based on experience. @omarArm , @thorstendb-ARM , please raise if reports for certain issues become more of a problem than a help.
Candidates to consider adding in future to improve code readability:
[smells.function_parameters][smells.nested_control_flow][smells.function_complexity]
For your information, several configurations related to the ones mentioned can be controlled. For example: |
Nice, setting a threshold looks handy. Does qlty also support code annotations for ignoring specific pieces of code you know that they will break guards? Similar to what eslint has? |
jreineckearm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates. Let's merge and refine after gathering some experience with it.
Yes, to some extent we can achieve this with annotation. Lets work on that in other PRs |
Changes