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/08/15 03:15:42 UTC

[GitHub] [spark] panbingkun opened a new pull request, #37516: [MINOR][CORE] Fix comment & code style in HadoopFSUtils

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

   
   ### What changes were proposed in this pull request?
   This PR fix comment & code style in HadoopFSUtils.
   
   ### Why are the changes needed?
   > 1.Eliminate user confusion
   > 2.Make the code style consistent
   
   ### Does this PR introduce _any_ user-facing change?
   No. except HadoopFSUtils.listLeafFiles
   
   ### How was this patch tested?
   Not needed. Existing unit tests pass.


-- 
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 #37516: [MINOR][CORE] Fix comment & code style in HadoopFSUtils

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


##########
core/src/main/scala/org/apache/spark/util/HadoopFSUtils.scala:
##########
@@ -187,10 +187,12 @@ private[spark] object HadoopFSUtils extends Logging {
 
   // scalastyle:off argcount
   /**
-   * Lists a single filesystem path recursively. If a `SparkContext` object is specified, this
-   * function may launch Spark jobs to parallelize listing based on `parallelismThreshold`.
+   * Lists a single filesystem path recursively.
+   *
+   * 1. If contextOpt is defined, this function may launch Spark jobs to parallelize listing
+   * based on `parallelismThreshold`.
    *
-   * If sessionOpt is None, this may be called on executors.
+   * 2. If contextOpt is None, this may be called on executors.

Review Comment:
   I don't think this makes the comments particulaily easier to read. The only valid fix here is the parameter name which is super trivial.



-- 
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 #37516: [MINOR][CORE] Fix comment & code style in HadoopFSUtils

Posted by GitBox <gi...@apache.org>.
srowen closed pull request #37516: [MINOR][CORE] Fix comment & code style in HadoopFSUtils
URL: https://github.com/apache/spark/pull/37516


-- 
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 #37516: [MINOR][CORE] Fix comment & code style in HadoopFSUtils

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


##########
core/src/main/scala/org/apache/spark/util/HadoopFSUtils.scala:
##########
@@ -57,14 +57,14 @@ private[spark] object HadoopFSUtils extends Logging {
    * @return for each input path, the set of discovered files for the path
    */
   def parallelListLeafFiles(
-    sc: SparkContext,
-    paths: Seq[Path],
-    hadoopConf: Configuration,
-    filter: PathFilter,
-    ignoreMissingFiles: Boolean,
-    ignoreLocality: Boolean,
-    parallelismThreshold: Int,
-    parallelismMax: Int): Seq[(Path, Seq[FileStatus])] = {
+      sc: SparkContext,
+      paths: Seq[Path],
+      hadoopConf: Configuration,
+      filter: PathFilter,
+      ignoreMissingFiles: Boolean,
+      ignoreLocality: Boolean,
+      parallelismThreshold: Int,
+      parallelismMax: Int): Seq[(Path, Seq[FileStatus])] = {

Review Comment:
   Let's don't fix the style arbitrarily. You would fix some styles when you fix other codes around. For example it makes backporting harder. 



-- 
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] AmplabJenkins commented on pull request #37516: [MINOR][CORE] Fix comment & code style in HadoopFSUtils

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

   Can one of the admins verify this patch?


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