You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/12/09 10:26:40 UTC

[GitHub] [spark] HyukjinKwon opened a new pull request, #39004: [SPARK-41466][BUILD] Change Scala Style configuration to catch AnyFunSuite instead of FunSuite

HyukjinKwon opened a new pull request, #39004:
URL: https://github.com/apache/spark/pull/39004

   ### What changes were proposed in this pull request?
   
   This PR is technically a followup of https://github.com/apache/spark/pull/29196 that changes Scala Style configuration to catch `AnyFunSuite` instead of `FunSuite`.
   
   ### Why are the changes needed?
   
   To keep the original intention of the Scala style configuration.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No, dev-only.
   
   ### How was this patch tested?
   
   Manually tested with `./dev/lint-scala`. CI in this PR should test it out too.


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a diff in pull request #39004: [SPARK-41466][BUILD] Change Scala Style configuration to catch AnyFunSuite instead of FunSuite

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39004:
URL: https://github.com/apache/spark/pull/39004#discussion_r1044347210


##########
scalastyle-config.xml:
##########
@@ -135,9 +135,9 @@ This file is divided into 3 sections:
   <!-- ??? usually shouldn't be checked into the code base. -->
   <check level="error" class="org.scalastyle.scalariform.NotImplementedErrorUsage" enabled="true"></check>
 
-  <!-- As of SPARK-7558, all tests in Spark should extend o.a.s.SparkFunSuite instead of FunSuite directly -->
+  <!-- As of SPARK-7558, all tests in Spark should extend o.a.s.SparkFunSuite instead of AnyFunSuite directly -->
   <check customId="funsuite" level="error" class="org.scalastyle.scalariform.TokenChecker" enabled="true">
-    <parameters><parameter name="regex">^FunSuite[A-Za-z]*$</parameter></parameters>
+    <parameters><parameter name="regex">^AnyFunSuite[A-Za-z]*$</parameter></parameters>

Review Comment:
   `FunSuite` actually doesn't exist anymore:
   
   > For example, org.scalatest.FunSuite was renamed to org.scalatest.funsuite.AnyFunSuite in 3.1.0.
   
   
   https://www.scalatest.org/release_notes/3.2.0



-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #39004: [SPARK-41466][BUILD] Change Scala Style configuration to catch AnyFunSuite instead of FunSuite

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39004:
URL: https://github.com/apache/spark/pull/39004#discussion_r1044356081


##########
scalastyle-config.xml:
##########
@@ -135,9 +135,9 @@ This file is divided into 3 sections:
   <!-- ??? usually shouldn't be checked into the code base. -->
   <check level="error" class="org.scalastyle.scalariform.NotImplementedErrorUsage" enabled="true"></check>
 
-  <!-- As of SPARK-7558, all tests in Spark should extend o.a.s.SparkFunSuite instead of FunSuite directly -->
+  <!-- As of SPARK-7558, all tests in Spark should extend o.a.s.SparkFunSuite instead of AnyFunSuite directly -->
   <check customId="funsuite" level="error" class="org.scalastyle.scalariform.TokenChecker" enabled="true">
-    <parameters><parameter name="regex">^FunSuite[A-Za-z]*$</parameter></parameters>
+    <parameters><parameter name="regex">^AnyFunSuite[A-Za-z]*$</parameter></parameters>

Review Comment:
   Oh, got it!



-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #39004: [SPARK-41466][BUILD] Change Scala Style configuration to catch AnyFunSuite instead of FunSuite

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39004:
URL: https://github.com/apache/spark/pull/39004#discussion_r1044325535


##########
scalastyle-config.xml:
##########
@@ -135,9 +135,9 @@ This file is divided into 3 sections:
   <!-- ??? usually shouldn't be checked into the code base. -->
   <check level="error" class="org.scalastyle.scalariform.NotImplementedErrorUsage" enabled="true"></check>
 
-  <!-- As of SPARK-7558, all tests in Spark should extend o.a.s.SparkFunSuite instead of FunSuite directly -->
+  <!-- As of SPARK-7558, all tests in Spark should extend o.a.s.SparkFunSuite instead of AnyFunSuite directly -->
   <check customId="funsuite" level="error" class="org.scalastyle.scalariform.TokenChecker" enabled="true">
-    <parameters><parameter name="regex">^FunSuite[A-Za-z]*$</parameter></parameters>
+    <parameters><parameter name="regex">^AnyFunSuite[A-Za-z]*$</parameter></parameters>

Review Comment:
   We need to detect and prevent both `FunSuite` and `AnyFunSuite` from new your PRs, don't we?



-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #39004: [SPARK-41466][BUILD] Change Scala Style configuration to catch AnyFunSuite instead of FunSuite

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #39004:
URL: https://github.com/apache/spark/pull/39004#issuecomment-1344576249

   Merged to master.


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun closed pull request #39004: [SPARK-41466][BUILD] Change Scala Style configuration to catch AnyFunSuite instead of FunSuite

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #39004: [SPARK-41466][BUILD] Change Scala Style configuration to catch AnyFunSuite instead of FunSuite
URL: https://github.com/apache/spark/pull/39004


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] amaliujia commented on pull request #39004: [SPARK-41466][BUILD] Change Scala Style configuration to catch AnyFunSuite instead of FunSuite

Posted by GitBox <gi...@apache.org>.
amaliujia commented on PR #39004:
URL: https://github.com/apache/spark/pull/39004#issuecomment-1344680268

   late LGTM thanks!


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #39004: [SPARK-41466][BUILD] Change Scala Style configuration to catch AnyFunSuite instead of FunSuite

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39004:
URL: https://github.com/apache/spark/pull/39004#discussion_r1044325535


##########
scalastyle-config.xml:
##########
@@ -135,9 +135,9 @@ This file is divided into 3 sections:
   <!-- ??? usually shouldn't be checked into the code base. -->
   <check level="error" class="org.scalastyle.scalariform.NotImplementedErrorUsage" enabled="true"></check>
 
-  <!-- As of SPARK-7558, all tests in Spark should extend o.a.s.SparkFunSuite instead of FunSuite directly -->
+  <!-- As of SPARK-7558, all tests in Spark should extend o.a.s.SparkFunSuite instead of AnyFunSuite directly -->
   <check customId="funsuite" level="error" class="org.scalastyle.scalariform.TokenChecker" enabled="true">
-    <parameters><parameter name="regex">^FunSuite[A-Za-z]*$</parameter></parameters>
+    <parameters><parameter name="regex">^AnyFunSuite[A-Za-z]*$</parameter></parameters>

Review Comment:
   We need to detect both `FunSuite` and `AnyFunSuite` from new your PRs, don't we?



-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org