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/02/10 12:34:17 UTC

[GitHub] [spark] LuciferYang opened a new pull request #35477: [SPARK-38175][SQL] Clean up unused parameters of `castDecimalToIntegralTypeCode` in `Cast.scala`

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


   ### What changes were proposed in this pull request?
   The private method `castDecimalToIntegralTypeCode` in `Cast.scala` has two unused input parameters, this pr clean up these.
   
   
   ### Why are the changes needed?
   Cleanup unused symbol.
   
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   
   ### How was this patch tested?
   Pass GA


-- 
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 change in pull request #35477: [SPARK-38175][CORE][SQL][SS][DSTREAM][MESOS][WEBUI] Clean up unused parameters in private methods signature

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35477:
URL: https://github.com/apache/spark/pull/35477#discussion_r804936182



##########
File path: streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingPage.scala
##########
@@ -340,8 +340,7 @@ private[ui] class StreamingPage(parent: StreamingTab)
       jsCollector: JsCollector,
       minX: Long,
       maxX: Long,
-      minY: Double,
-      maxY: Double): Seq[Node] = {
+      minY: Double): Seq[Node] = {

Review comment:
       Got it. We use `maxYCalculated` instead of `maxY`. 




-- 
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 edited a comment on pull request #35477: [SPARK-38175][SQL] Clean up unused parameters of `castDecimalToIntegralTypeCode` in `Cast.scala`

Posted by GitBox <gi...@apache.org>.
LuciferYang edited a comment on pull request #35477:
URL: https://github.com/apache/spark/pull/35477#issuecomment-1035906671


   My idea is to add the `- wunused: params` option for compilation, then analyze the compilation log to find `unused parameters in private methods` in Spark code and fix them, It will take some time to complete the work because the compilation log  also includes `unused parameters in non private methods` @srowen @dongjoon-hyun 
   
   
   


-- 
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 #35477: [SPARK-38175][CORE][SQL][SS][DSTREAM][MESOS][WEBUI] Clean up unused parameters in private methods signature

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35477:
URL: https://github.com/apache/spark/pull/35477#issuecomment-1038572224


   thanks all ~


-- 
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 #35477: [SPARK-38175][SQL] Clean up unused parameters of `castDecimalToIntegralTypeCode` in `Cast.scala`

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35477:
URL: https://github.com/apache/spark/pull/35477#issuecomment-1035906671


   My idea is to add the `- wunused: params' option for compilation, then analyze the compilation log to find `unused parameters in private methods` in Spark code and fix them, It will take some time to complete the work because the compilation log  also includes `unused parameters in non private methods` @srowen @dongjoon-hyun 
   
   
   


-- 
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 #35477: [SPARK-38175][SQL] Clean up unused parameters of private methods

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35477:
URL: https://github.com/apache/spark/pull/35477#issuecomment-1036156572


   There are still some cases to be checked
   
   


-- 
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 #35477: [SPARK-38175][SQL] Clean up unused parameters of `castDecimalToIntegralTypeCode` in `Cast.scala`

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35477:
URL: https://github.com/apache/spark/pull/35477#issuecomment-1035809965


   > Looks fine. Are there any other such simplifications available here in private methods?
   
   I think there should be other similar cases, let me try to find and fix 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] LuciferYang commented on a change in pull request #35477: [SPARK-38175][CORE][SQL][SS][DSTREAM][MESOS][WEBUI] Clean up unused parameters in private methods signature

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35477:
URL: https://github.com/apache/spark/pull/35477#discussion_r805099303



##########
File path: core/src/test/scala/org/apache/spark/SparkContextSchedulerCreationSuite.scala
##########
@@ -32,23 +32,18 @@ class SparkContextSchedulerCreationSuite
   def noOp(taskSchedulerImpl: TaskSchedulerImpl): Unit = {}
 
   def createTaskScheduler(master: String)(body: TaskSchedulerImpl => Unit = noOp): Unit =
-    createTaskScheduler(master, "client")(body)
-
-  def createTaskScheduler(master: String, deployMode: String)(
-      body: TaskSchedulerImpl => Unit): Unit =
-    createTaskScheduler(master, deployMode, new SparkConf())(body)
+    createTaskScheduler(master, new SparkConf())(body)
 
   def createTaskScheduler(
       master: String,
-      deployMode: String,
       conf: SparkConf)(body: TaskSchedulerImpl => Unit): Unit = {
     // Create local SparkContext to setup a SparkEnv. We don't actually want to start() the
     // real schedulers, so we don't want to create a full SparkContext with the desired scheduler.
     sc = new SparkContext("local", "test", conf)
     val createTaskSchedulerMethod =
       PrivateMethod[Tuple2[SchedulerBackend, TaskScheduler]](Symbol("createTaskScheduler"))
     val (_, sched) =
-      SparkContext invokePrivate createTaskSchedulerMethod(sc, master, deployMode)
+      SparkContext invokePrivate createTaskSchedulerMethod(sc, master)

Review comment:
       Change this file because the reflection call to private method `SparkContext.createTaskScheduler` 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] dongjoon-hyun closed pull request #35477: [SPARK-38175][CORE][SQL][SS][DSTREAM][MESOS][WEBUI] Clean up unused parameters in private methods signature

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #35477:
URL: https://github.com/apache/spark/pull/35477


   


-- 
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 change in pull request #35477: [SPARK-38175][CORE][SQL][SS][DSTREAM][MESOS][WEBUI] Clean up unused parameters in private methods signature

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35477:
URL: https://github.com/apache/spark/pull/35477#discussion_r804936754



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/FileStreamOptions.scala
##########
@@ -33,9 +33,9 @@ class FileStreamOptions(parameters: CaseInsensitiveMap[String]) extends Logging
 
   def this(parameters: Map[String, String]) = this(CaseInsensitiveMap(parameters))
 
-  checkDisallowedOptions(parameters)
+  checkDisallowedOptions()
 
-  private def checkDisallowedOptions(options: Map[String, String]): Unit = {
+  private def checkDisallowedOptions(): Unit = {
     Seq(ModifiedBeforeFilter.PARAM_NAME, ModifiedAfterFilter.PARAM_NAME).foreach { param =>
       if (parameters.contains(param)) {

Review comment:
       It seems that the original design wanted to use `options` here. However, `CaseInsensitiveMap` and `Map` is different. So, I agree with this new change.




-- 
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 #35477: [SPARK-38175][CORE][SQL][SS][DSTREAM][MESOS][WEBUI] Clean up unused parameters in private methods signature

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


   Could you re-trigger GitHub Action?


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