-
Notifications
You must be signed in to change notification settings - Fork 14.7k
KAFKA-19809 CheckStyle version upgrade: 10.20.2 -->> 12.0.1 #20726
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
base: trunk
Are you sure you want to change the base?
Conversation
| <module name="ClassDataAbstractionCoupling"> | ||
| <!-- default is 7 --> | ||
| <property name="max" value="25"/> | ||
| <property name="max" value="27"/> |
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.
Options:
- increase limit (like here)
- create a baseline (skip/suppress this check for existing cases/classes)
- change existing classes and keep the old property value max limit
| <module name="ClassFanOutComplexity"> | ||
| <!-- default is 20 --> | ||
| <property name="max" value="52"/> | ||
| <property name="max" value="55"/> |
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.
In a similar case, we also have three options:
- increase limit (like here)
- create a baseline (skip/suppress this check for existing cases/classes)
- change existing classes and keep the old property value max limit
| <suppress checks="ImportControl" files="(JaasTestUtils).java" /> | ||
|
|
||
| <suppress checks="FinalLocalVariable" files="."/> | ||
| <suppress checks="FinalParameters" files="."/> |
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.
From my POV both of these checks are overzealous and hence I turned them off, but I'm open to a discussion.
Related links:
- https://checkstyle.sourceforge.io/checks/misc/finalparameters.html
- https://checkstyle.sourceforge.io/checks/coding/finallocalvariable.html
- https://softwareengineering.stackexchange.com/questions/48413/in-java-should-i-use-final-for-parameters-and-locals-even-when-i-dont-have-t
- https://stackoverflow.com/questions/3635863/why-method-parameters-needs-to-be-set-as-final
- https://stackoverflow.com/questions/500508/why-should-i-use-the-keyword-final-on-a-method-parameter-in-java
- https://www.reddit.com/r/learnjava/comments/1ffoa9p/using_final_in_method_parameters_your_opinion
|
Can you please take a look @chia7712 ? |
Jira ticket: KAFKA-19809
Related link: https://checkstyle.org/releasenotes.html#Release_12.0.1
Reviews are welcome!
Relates to: checkstyle/checkstyle#16003