You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/05/09 15:43:42 UTC

[GitHub] [kafka] dejan2609 opened a new pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

dejan2609 opened a new pull request #10658:
URL: https://github.com/apache/kafka/pull/10658


   ⚠️  Not meant to be merged.  Related to https://github.com/checkstyle/checkstyle/issues/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).
   
   **_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:_** 
   - https://github.com/dejan2609/kafka/blob/apache-kafka-with-checkstyle-8.42/build.gradle#L588
   - https://github.com/dejan2609/kafka/blob/apache-kafka-with-checkstyle-8.42/build.gradle#L712
   ```
   build.gradle
   gradle/dependencies.gradle
   /chekstyle/ folder (with checkstyle config files)
   ```
   
   ℹ️ Note: also related to this Kafka PR: #10656
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-841828282


   Thanx for info @romani, much appreciated !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 edited a comment on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 edited a comment on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-836815768


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] romani commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
romani commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-841823518


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] romani commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
romani commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-835929950


   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-836815768


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] romani commented on a change in pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
romani commented on a change in pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#discussion_r628966279



##########
File path: checkstyle/checkstyle.xml
##########
@@ -91,7 +91,7 @@
       <property name="allowEmptyConstructors" value="true"/>
       <property name="allowEmptyMethods" value="true"/>
     </module>
-    <module name="Indentation"/>
+    <!-- <module name="Indentation"/> -->

Review comment:
       Ok, thanks.
   But we will chase all Checks violations.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-841795654


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-840084223


   Also... both here and on my laptop there is another type of problem with some test failing like this: 
   
   ```
   org.apache.directory.api.ldap.schema.extractor.UniqueResourceException: Problem locating LDIF file in schema repository
   Multiple copies of resource named 'schema/ou=schema/cn=apachedns/ou=objectclasses/m-oid=1.3.6.1.4.1.18060.0.4.2.3.9.ldif' located on classpath at urls
       jarːfileː/home/jenkins/.gradle/caches/modules-2/files-2.1/org.apache.directory.api/api-all/1.0.2/5a0f957dfe453ecaa6e7cbb19d2698083116e123/api-all-1.0.2.jar!/schema/ou%3dschema/cn%3dapachedns/ou%3dobjectclasses/m-oid%3d1.3.6.1.4.1.18060.0.4.2.3.9.ldif
       jarːfileː/home/jenkins/.gradle/caches/modules-2/files-2.1/org.apache.directory.api/api-ldap-schema-data/1.0.0/eca194f6baca671c466a4eb3ebc669856d997baa/api-ldap-schema-data-1.0.0.jar!/schema/ou%3dschema/cn%3dapachedns/ou%3dobjectclasses/m-oid%3d1.3.6.1.4.1.18060.0.4.2.3.9.ldif
   
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 removed a comment on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 removed a comment on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-840084223


   Also... both here and on my laptop there is another type of problem with some test failing like this: 
   
   ```
   org.apache.directory.api.ldap.schema.extractor.UniqueResourceException: Problem locating LDIF file in schema repository
   Multiple copies of resource named 'schema/ou=schema/cn=apachedns/ou=objectclasses/m-oid=1.3.6.1.4.1.18060.0.4.2.3.9.ldif' located on classpath at urls
       jarːfileː/home/jenkins/.gradle/caches/modules-2/files-2.1/org.apache.directory.api/api-all/1.0.2/5a0f957dfe453ecaa6e7cbb19d2698083116e123/api-all-1.0.2.jar!/schema/ou%3dschema/cn%3dapachedns/ou%3dobjectclasses/m-oid%3d1.3.6.1.4.1.18060.0.4.2.3.9.ldif
       jarːfileː/home/jenkins/.gradle/caches/modules-2/files-2.1/org.apache.directory.api/api-ldap-schema-data/1.0.0/eca194f6baca671c466a4eb3ebc669856d997baa/api-ldap-schema-data-1.0.0.jar!/schema/ou%3dschema/cn%3dapachedns/ou%3dobjectclasses/m-oid%3d1.3.6.1.4.1.18060.0.4.2.3.9.ldif
   
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-835953405


   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). 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 closed pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 closed pull request #10658:
URL: https://github.com/apache/kafka/pull/10658


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 closed pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 closed pull request #10658:
URL: https://github.com/apache/kafka/pull/10658


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-840092184


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-841795654






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 commented on a change in pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 commented on a change in pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#discussion_r628963747



##########
File path: checkstyle/checkstyle.xml
##########
@@ -91,7 +91,7 @@
       <property name="allowEmptyConstructors" value="true"/>
       <property name="allowEmptyMethods" value="true"/>
     </module>
-    <module name="Indentation"/>
+    <!-- <module name="Indentation"/> -->

Review comment:
       @romani  this module is excluded for your convenience in order to suppress `Indentation` errors that are insignificant in this context (because we are hunting down only `UnnecessaryParentheses` regression).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dejan2609 commented on a change in pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
dejan2609 commented on a change in pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#discussion_r629430530



##########
File path: checkstyle/checkstyle.xml
##########
@@ -91,7 +91,7 @@
       <property name="allowEmptyConstructors" value="true"/>
       <property name="allowEmptyMethods" value="true"/>
     </module>
-    <module name="Indentation"/>
+    <!-- <module name="Indentation"/> -->

Review comment:
       Roger that @romani. 
   
   Ok, I will revoke this module exclusion than (and then you will be able receive **_all_** Kafka's CheckStyle 8.42 violations). 
   
   I will also rebase this PR onto trunk (CheckStyle upgrade from 8.20 to 8.36.2 was merged to trunk today).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] romani commented on pull request #10658: POC for CheckStyle 8.42 regression (with 'Unnecessary Parentheses' errors)

Posted by GitBox <gi...@apache.org>.
romani commented on pull request #10658:
URL: https://github.com/apache/kafka/pull/10658#issuecomment-841823518


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org