You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/07/07 09:43:10 UTC

[GitHub] [ratis] leo65535 opened a new pull request, #674: RATIS-1616. Include test source to check

leo65535 opened a new pull request, #674:
URL: https://github.com/apache/ratis/pull/674

   ## What changes were proposed in this pull request?
   
   Include test source to check.
   
   ## What is the link to the Apache JIRA
   
   (Please create an issue in ASF JIRA before opening a pull request,
   and you need to set the title of the pull request which starts with
   the corresponding JIRA issue number. (e.g. RATIS-XXXX. Fix a typo in YYY.)
   
   Please replace this section with the link to the Apache JIRA)
   
   ## How was this patch tested?
   
   (Please explain how this patch was tested. Ex: unit tests, manual tests)
   (If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)
   


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

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

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


[GitHub] [ratis] leo65535 commented on a diff in pull request #674: RATIS-1616. Include test source to check

Posted by GitBox <gi...@apache.org>.
leo65535 commented on code in PR #674:
URL: https://github.com/apache/ratis/pull/674#discussion_r916386221


##########
pom.xml:
##########
@@ -769,6 +769,7 @@
         <configuration>
           <configLocation>dev-support/checkstyle.xml</configLocation>
           <failOnViolation>false</failOnViolation>
+          <includeTestSourceDirectory>true</includeTestSourceDirectory>

Review Comment:
   Make sense. But it's better to keep the same rules for both `source` and `test` code.



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

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

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


[GitHub] [ratis] leo65535 commented on a diff in pull request #674: RATIS-1616. Include test source to check

Posted by GitBox <gi...@apache.org>.
leo65535 commented on code in PR #674:
URL: https://github.com/apache/ratis/pull/674#discussion_r916386221


##########
pom.xml:
##########
@@ -769,6 +769,7 @@
         <configuration>
           <configLocation>dev-support/checkstyle.xml</configLocation>
           <failOnViolation>false</failOnViolation>
+          <includeTestSourceDirectory>true</includeTestSourceDirectory>

Review Comment:
   Make sense. But it's better to keep the same rules for both `source` and `test` code. Developers will always meet the source code check style error, but test code is ok, this is not friendly for developers.
   
   



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

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

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


[GitHub] [ratis] szetszwo commented on a diff in pull request #674: RATIS-1616. Include test source to check

Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #674:
URL: https://github.com/apache/ratis/pull/674#discussion_r916151232


##########
pom.xml:
##########
@@ -769,6 +769,7 @@
         <configuration>
           <configLocation>dev-support/checkstyle.xml</configLocation>
           <failOnViolation>false</failOnViolation>
+          <includeTestSourceDirectory>true</includeTestSourceDirectory>

Review Comment:
   @leo65535 , thanks for working on this.  Fixing the checkstyle warnings in test is good.  However, let's don't run checkstyle for the tests.  We should focus more on the main code instead of spending too much energy on the tests.



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

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

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


[GitHub] [ratis] leo65535 closed pull request #674: RATIS-1616. Include test source to check

Posted by "leo65535 (via GitHub)" <gi...@apache.org>.
leo65535 closed pull request #674: RATIS-1616. Include test source to check
URL: https://github.com/apache/ratis/pull/674


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

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

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