You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "HyukjinKwon (via GitHub)" <gi...@apache.org> on 2023/09/22 05:24:04 UTC

[GitHub] [spark] HyukjinKwon opened a new pull request, #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

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

   ### What changes were proposed in this pull request?
   
   This PR is a followup of https://github.com/apache/spark/pull/43008 that cleanups Scala version specific comments, and `scala-2.13` profile usage.
   
   See also https://github.com/apache/spark/pull/43008#discussion_r1333866152
   
   ### Why are the changes needed?
   
   To remove unnecessary profile specifications
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   CI in this PR should test them out. For README.md, I manually checked via GitHub.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No.
   


-- 
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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333896190


##########
pom.xml:
##########
@@ -3648,23 +3646,6 @@
       </properties>
     </profile>
 
-    <profile>
-      <id>scala-2.13</id>

Review Comment:
   We still need this because we need a way to switch back to 2.13 if we switch to Scala 3 profile (like we do before).



-- 
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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #43049:
URL: https://github.com/apache/spark/pull/43049#issuecomment-1730832844

   BTW, it's surprising to me that the diff size is very small.
   ![Screenshot 2023-09-21 at 10 42 52 PM](https://github.com/apache/spark/assets/9700541/8ecea687-8787-4d3a-9670-1c1dd4821ad5)
   


-- 
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] LuciferYang commented on a diff in pull request #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333898761


##########
dev/mima:
##########
@@ -24,9 +24,9 @@ set -e
 FWDIR="$(cd "`dirname "$0"`"/..; pwd)"
 cd "$FWDIR"
 
-SPARK_PROFILES=${1:-"-Pscala-2.13 -Pmesos -Pkubernetes -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Phive-thriftserver -Phive"}
-TOOLS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false "export tools/fullClasspath" | grep jar | tail -n1)"
-OLD_DEPS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false $SPARK_PROFILES "export oldDeps/fullClasspath" | grep jar | tail -n1)"

Review Comment:
   While cleaning this up, I suggest removing `2.13` from `VALID_VERSIONS` in the `change-scala-version.sh` script too, otherwise if we execute `dev/change-scala-version.sh 2.13`, these files will generate some unnecessary git diff.
   
   https://github.com/apache/spark/blob/5f643ee1444d41ca78ef397d9675efac550a6967/dev/change-scala-version.sh#L22



-- 
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] LuciferYang commented on a diff in pull request #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333898761


##########
dev/mima:
##########
@@ -24,9 +24,9 @@ set -e
 FWDIR="$(cd "`dirname "$0"`"/..; pwd)"
 cd "$FWDIR"
 
-SPARK_PROFILES=${1:-"-Pscala-2.13 -Pmesos -Pkubernetes -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Phive-thriftserver -Phive"}
-TOOLS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false "export tools/fullClasspath" | grep jar | tail -n1)"
-OLD_DEPS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false $SPARK_PROFILES "export oldDeps/fullClasspath" | grep jar | tail -n1)"

Review Comment:
   While cleaning this up, I suggest removing `2.13` from `VALID_VERSIONS` in the `change-scala-version.sh` script, otherwise if we execute `dev/change-scala-version.sh 2.13`, these files will generate some unnecessary git diff.
   
   https://github.com/apache/spark/blame/5f643ee1444d41ca78ef397d9675efac550a6967/dev/change-scala-version.sh#L22



-- 
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 #43049: [SPARK-45272][BUILD][INFRA][DOCS] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333903491


##########
pom.xml:
##########
@@ -3648,13 +3646,14 @@
       </properties>
     </profile>
 
+    <!--
+     SPARK-34774 Add this property to ensure change-scala-version.sh can replace the public `scala.version`
+     property correctly.
+    -->

Review Comment:
   Nesting a comment doesn't work .. so I took it out.



-- 
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] LuciferYang commented on a diff in pull request #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333889711


##########
connector/avro/pom.xml:
##########
@@ -70,12 +70,10 @@
       <groupId>org.apache.spark</groupId>
       <artifactId>spark-tags_${scala.binary.version}</artifactId>
     </dependency>
-    <!-- #if scala-2.13 -->

Review Comment:
   Are we sure about deleting these comments? I was hesitant before, because I didn't know if we would need to use a similar approach to dynamically change dependencies when supporting Scala 3, so I took a conservative approach and kept them.



-- 
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 pull request #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #43049:
URL: https://github.com/apache/spark/pull/43049#issuecomment-1730819193

   I didn't cleanup `dev/lint-scala` because the Scala profile was there even before that PR.


-- 
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] LuciferYang commented on a diff in pull request #43049: [SPARK-45272][BUILD][INFRA][DOCS] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333907049


##########
connector/avro/pom.xml:
##########
@@ -70,12 +70,10 @@
       <groupId>org.apache.spark</groupId>
       <artifactId>spark-tags_${scala.binary.version}</artifactId>
     </dependency>
-    <!-- #if scala-2.13 -->

Review Comment:
   It seems that for Scala 3 the `scala.binary.version` is always `3`? Then it looks like we can delete this comment, the corresponding dependency for Scala 3 is
   
   ```
   <dependency>
       <groupId>org.scala-lang.modules</groupId>
       <artifactId>scala-parallel-collections_3</artifactId>
       <version>1.0.4</version>
   </dependency>
   ```
   It seems that this dependency no longer needs to be dynamically changed.
   
   



-- 
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 pull request #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #43049:
URL: https://github.com/apache/spark/pull/43049#issuecomment-1730819584

   and I am intentionally using the same JIRA to make it easier to manage together (e.g., reverting).
   
   cc @dongjoon-hyun and @LuciferYang FYI


-- 
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 #43049: [SPARK-45272][BUILD][INFRA][DOCS] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #43049: [SPARK-45272][BUILD][INFRA][DOCS] Remove Scala version specific comments, and scala-2.13 profile usage
URL: https://github.com/apache/spark/pull/43049


-- 
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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333896833


##########
pom.xml:
##########
@@ -3648,23 +3646,6 @@
       </properties>
     </profile>
 
-    <profile>
-      <id>scala-2.13</id>

Review Comment:
   I will comment this out.



-- 
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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333901878


##########
dev/mima:
##########
@@ -24,9 +24,9 @@ set -e
 FWDIR="$(cd "`dirname "$0"`"/..; pwd)"
 cd "$FWDIR"
 
-SPARK_PROFILES=${1:-"-Pscala-2.13 -Pmesos -Pkubernetes -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Phive-thriftserver -Phive"}
-TOOLS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false "export tools/fullClasspath" | grep jar | tail -n1)"
-OLD_DEPS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false $SPARK_PROFILES "export oldDeps/fullClasspath" | grep jar | tail -n1)"

Review Comment:
   I commented `dev/change-scala-version.sh` usage out in the README.md :-).



-- 
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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333895946


##########
connector/avro/pom.xml:
##########
@@ -70,12 +70,10 @@
       <groupId>org.apache.spark</groupId>
       <artifactId>spark-tags_${scala.binary.version}</artifactId>
     </dependency>
-    <!-- #if scala-2.13 -->

Review Comment:
   Do we need to change `scala-parallel-collections` for Scala 3 upgrade? I would think we'd need it for other libraries. I am not removing the mechanism.



-- 
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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #43049:
URL: https://github.com/apache/spark/pull/43049#issuecomment-1730826061

   I have the same concern. I'd like to keep these because of Scala 3.


-- 
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] LuciferYang commented on a diff in pull request #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333897015


##########
pom.xml:
##########
@@ -3648,23 +3646,6 @@
       </properties>
     </profile>
 
-    <profile>
-      <id>scala-2.13</id>

Review Comment:
   +1



-- 
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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #43049:
URL: https://github.com/apache/spark/pull/43049#issuecomment-1730835682

   @HyukjinKwon  and @LuciferYang .
   
   If the proposed removal is only 53 lines, I believe this could be a cleaner way and we can recover it back easily.
   
   To recover only this, shall we use an independent JIRA issue, @HyukjinKwon ?


-- 
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 pull request #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #43049:
URL: https://github.com/apache/spark/pull/43049#issuecomment-1730836146

   Sure, I am fine


-- 
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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333894185


##########
dev/mima:
##########
@@ -24,9 +24,9 @@ set -e
 FWDIR="$(cd "`dirname "$0"`"/..; pwd)"
 cd "$FWDIR"
 
-SPARK_PROFILES=${1:-"-Pscala-2.13 -Pmesos -Pkubernetes -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Phive-thriftserver -Phive"}
-TOOLS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false "export tools/fullClasspath" | grep jar | tail -n1)"
-OLD_DEPS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false $SPARK_PROFILES "export oldDeps/fullClasspath" | grep jar | tail -n1)"

Review Comment:
   ~This is good. What I conceded is only `pom.xml` file changes.~ Oh, sorry. I missed that this is also directly related.



-- 
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] LuciferYang commented on a diff in pull request #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333889711


##########
connector/avro/pom.xml:
##########
@@ -70,12 +70,10 @@
       <groupId>org.apache.spark</groupId>
       <artifactId>spark-tags_${scala.binary.version}</artifactId>
     </dependency>
-    <!-- #if scala-2.13 -->

Review Comment:
   Are we sure about deleting these comments? I was hesitant before, because I didn't know if we would need to use a similar approach to dynamically modify dependencies when supporting Scala 3, so I took a conservative approach and kept them.



-- 
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 #43049: [SPARK-45272][BUILD][INFRA][DOCS] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #43049:
URL: https://github.com/apache/spark/pull/43049#issuecomment-1730910184

   Many builds successful including Java 21 Maven build. Let me merge 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: 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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333894185


##########
dev/mima:
##########
@@ -24,9 +24,9 @@ set -e
 FWDIR="$(cd "`dirname "$0"`"/..; pwd)"
 cd "$FWDIR"
 
-SPARK_PROFILES=${1:-"-Pscala-2.13 -Pmesos -Pkubernetes -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Phive-thriftserver -Phive"}
-TOOLS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false "export tools/fullClasspath" | grep jar | tail -n1)"
-OLD_DEPS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false $SPARK_PROFILES "export oldDeps/fullClasspath" | grep jar | tail -n1)"

Review Comment:
   This is good. What I conceded is only `pom.xml` file changes, @HyukjinKwon .



-- 
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 #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333898023


##########
pom.xml:
##########
@@ -3648,23 +3646,6 @@
       </properties>
     </profile>
 
-    <profile>
-      <id>scala-2.13</id>

Review Comment:
   +1



-- 
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] LuciferYang commented on a diff in pull request #43049: [SPARK-44113][BUILD][INFRA][DOCS][FOLLOW-UP] Remove Scala version specific comments, and scala-2.13 profile usage

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43049:
URL: https://github.com/apache/spark/pull/43049#discussion_r1333898761


##########
dev/mima:
##########
@@ -24,9 +24,9 @@ set -e
 FWDIR="$(cd "`dirname "$0"`"/..; pwd)"
 cd "$FWDIR"
 
-SPARK_PROFILES=${1:-"-Pscala-2.13 -Pmesos -Pkubernetes -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Phive-thriftserver -Phive"}
-TOOLS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false "export tools/fullClasspath" | grep jar | tail -n1)"
-OLD_DEPS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false $SPARK_PROFILES "export oldDeps/fullClasspath" | grep jar | tail -n1)"

Review Comment:
   While cleaning this up, I suggest removing `2.13` from `VALID_VERSIONS` in the `change-scala-version.sh` script, otherwise if we execute `dev/change-scala-version.sh 2.13`, these files will generate some unnecessary git diff.
   
   https://github.com/apache/spark/blob/5f643ee1444d41ca78ef397d9675efac550a6967/dev/change-scala-version.sh#L22



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