Skip to content

POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)#10658

Closed
dejan2609 wants to merge 1 commit intoapache:trunkfrom
dejan2609:apache-kafka-with-checkstyle-8.42
Closed

POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)#10658
dejan2609 wants to merge 1 commit intoapache:trunkfrom
dejan2609:apache-kafka-with-checkstyle-8.42

Conversation

@dejan2609
Copy link
Copy Markdown
Contributor

@dejan2609 dejan2609 commented May 9, 2021

⚠️ Not meant to be merged. Related to checkstyle/checkstyle#9957

@romani please note that I intentionally fine-grained this PR to a few commits (with appropriate messages, I hope you will find them useful). Rationale: my intention was to dissect important parts bit-by-bit (let me know in case you want me to change something).

Notable temporary changes:

  • checkstyle gradle maxError property is introduced (just for counting error counting purposes)
  • module Indentation is disable via checkstyle.xml (in this contex those errors are not relevant)

How to execute CheckStyle gradle tasks: ./gradlew checkstyleMain checkstyleTest
https://github.com/dejan2609/kafka/blob/apache-kafka-with-checkstyle-8.42/README.md#running-code-quality-checks

CheckStyle gradle configuration / config files:

build.gradle
gradle/dependencies.gradle
/chekstyle/ folder (with checkstyle config files)

ℹ️ Note: also related to this Kafka PR: #10656

@romani
Copy link
Copy Markdown

romani commented May 9, 2021

Unfortunately we need zero violations to be enforced. Only in this case we reliability can detect regression. Is it doable ? If team allow violations in mid of iteration, it is fine, at least we will use strict version of code where 0 violation.

@dejan2609
Copy link
Copy Markdown
Contributor Author

Please note that Kafka project (trunk branch, that is) does not declare maxErrors property at all (i.e. that value defaults to zero).

As I mentioned via previous commit messages I just temporary declared maxErrors property/value pair for this PR in order to count how many errors we have to deal with.

Unfortunately we need zero violations to be enforced. Only in this case we reliability can detect regression. Is it doable ?

Done, commit is pushed (property removed).

Comment thread checkstyle/checkstyle.xml Outdated
notes: you will find following Checkstyle violations/errors:
 * swarm of false positives for [UnnecessaryParentheses]
 * around 50 errors for [Indentation]
 * and just a few of [WhitespaceAround] errors
@dejan2609 dejan2609 force-pushed the apache-kafka-with-checkstyle-8.42 branch from a39300f to 4675622 Compare May 10, 2021 15:06
@dejan2609
Copy link
Copy Markdown
Contributor Author

dejan2609 commented May 10, 2021

Rebased onto trunk and force-pushed (as explained in a conversation/review above).

@dejan2609
Copy link
Copy Markdown
Contributor Author

Note: comment (related to a different PR) was accidentally submitted here and then deleted... sorry for spamming.

@dejan2609
Copy link
Copy Markdown
Contributor Author

(Will be) solved elsewhere and hence closing in favor of #10698

@dejan2609 dejan2609 closed this May 16, 2021
@romani
Copy link
Copy Markdown

romani commented May 16, 2021

@dejan2609 , fix for checkstyle/checkstyle#9957 will be released in 2 weeks.

@dejan2609
Copy link
Copy Markdown
Contributor Author

Thanx for info @romani, much appreciated !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants