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

[GitHub] [spark] LuciferYang opened a new pull request, #41365: [SPARK-43863][BUILD] Remove redundant `toSeq` from `SparkConnectPlanner` for Scala 2.13

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

   ### What changes were proposed in this pull request?
   This pr aims to remove redundant `toSeq` from `SparkConnectPlanner` for Scala 2.13.
   
   ### Why are the changes needed?
   Remove redundant `toSeq` for Scala 2.13 to avoid unnecessary collection copies
   
   
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   - Pass Github Actions
   - Manually checked `connect` and `connect-client-jvm` module for Scala 2.12


-- 
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] srowen commented on pull request #41365: [SPARK-43863][CONNECT] Remove redundant `toSeq` from `SparkConnectPlanner` for Scala 2.13

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

   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] LuciferYang commented on pull request #41365: [SPARK-43863][CONNECT] Remove redundant `toSeq` from `SparkConnectPlanner` for Scala 2.13

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

   For Scala 2.12, there are over 50 redundant `toSeq` in `SparkConnectPlanner`, I have selected 14 of them, which are also redundant `toSeq` for Scala 2.13.
   
   These changes are all for local vals and I think they should be safe, and I have also manually verified the testing of Scala 2.12, they passed.
   
   
   


-- 
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] srowen commented on pull request #41365: [SPARK-43863][BUILD] Remove redundant `toSeq` from `SparkConnectPlanner` for Scala 2.13

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

   The only question will be whether this was somehow needed in Scala 2.12. I'm always slightly worried that the copy is accidentally needed, but I doubt it here


-- 
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] srowen closed pull request #41365: [SPARK-43863][CONNECT] Remove redundant `toSeq` from `SparkConnectPlanner` for Scala 2.13

Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen closed pull request #41365: [SPARK-43863][CONNECT] Remove redundant `toSeq` from `SparkConnectPlanner` for Scala 2.13
URL: https://github.com/apache/spark/pull/41365


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