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 <gi...@git.apache.org> on 2016/04/01 12:06:47 UTC
[GitHub] spark pull request: [MINOR][SQL] Fix comments style and corrects s...
GitHub user HyukjinKwon opened a pull request:
https://github.com/apache/spark/pull/12109
[MINOR][SQL] Fix comments style and corrects several styles in CSV data source
## What changes were proposed in this pull request?
While trying to create a PR (which was not an issue at the end), I just corrected some style nits.
So, I removed the changes except for some coding style corrections.
- According to the [scala-style-guide#documentation-style](https://github.com/databricks/scala-style-guide#documentation-style), Scala style comments are discouraged.
>```scala
>/** This is a correct one-liner, short description. */
>
>/**
> * This is correct multi-line JavaDoc comment. And
> * this is my second line, and if I keep typing, this would be
> * my third line.
> */
>
>/** In Spark, we don't use the ScalaDoc style so this
> * is not correct.
> */
>```
- According to [scala-style-guide#blank-lines-vertical-whitespace](https://github.com/databricks/scala-style-guide#blank-lines-vertical-whitespace), single newline appears when
>Between consecutive members (or initializers) of a class: fields, constructors, methods, nested classes, static initializers, instance initializers.
So, this PR also removes the line.
- Remove uesless parentheses in tests
- Use `mapPartitions` instead of `mapPartitionsWithIndex()`.
## How was this patch tested?
Unit tests were used and `dev/run_tests` for style tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HyukjinKwon/spark SPARK-14271
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12109.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #12109
----
commit c8f1745689d82b6b0cc714ac326ce8cf65888f61
Author: hyukjinkwon <gu...@gmail.com>
Date: 2016-04-01T08:58:29Z
Support for reading with rowSeparator option
commit b8f09347cfe4aa7f371d4964a0452ed711d82bfb
Author: hyukjinkwon <gu...@gmail.com>
Date: 2016-04-01T09:46:26Z
Reverts changes
commit 784d8f7ba3120c9873517d3137284e44ebe5ccca
Author: hyukjinkwon <gu...@gmail.com>
Date: 2016-04-01T09:48:45Z
Java style comments
commit 878ae8911480ca48e0dd5ed7d10fff2670d97514
Author: hyukjinkwon <gu...@gmail.com>
Date: 2016-04-01T09:50:25Z
Revert other changes as well and correct style
commit f56e748ed81711a98d70427b791a790f26da471d
Author: hyukjinkwon <gu...@gmail.com>
Date: 2016-04-01T09:51:34Z
Revert toString() to toString
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [MINOR][SQL] Fix comments styl and correct sev...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/12109#discussion_r58288525
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVRelation.scala ---
@@ -42,12 +42,12 @@ object CSVRelation extends Logging {
firstLine: String,
params: CSVOptions): RDD[Array[String]] = {
// If header is set, make sure firstLine is materialized before sending to executors.
- file.mapPartitionsWithIndex({
- case (split, iter) => new BulkCsvReader(
+ file.mapPartitions { iter =>
--- End diff --
Oh, actually I noticed that there are not so many of them. I can change all the comments and maybe I could add a pattern.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [MINOR][SQL] Fix comments style and corrects s...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12109#issuecomment-204337478
**[Test build #54698 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54698/consoleFull)** for PR 12109 at commit [`f56e748`](https://github.com/apache/spark/commit/f56e748ed81711a98d70427b791a790f26da471d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [MINOR][SQL] Fix comments styl and correct sev...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12109#issuecomment-204358491
**[Test build #54698 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54698/consoleFull)** for PR 12109 at commit [`f56e748`](https://github.com/apache/spark/commit/f56e748ed81711a98d70427b791a790f26da471d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [MINOR][SQL] Fix comments styl and correct sev...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12109#issuecomment-204652108
The doc changes are actually good. Going to merge this in master. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [MINOR][SQL] Fix comments styl and correct sev...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/12109#discussion_r58288165
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVRelation.scala ---
@@ -42,12 +42,12 @@ object CSVRelation extends Logging {
firstLine: String,
params: CSVOptions): RDD[Array[String]] = {
// If header is set, make sure firstLine is materialized before sending to executors.
- file.mapPartitionsWithIndex({
- case (split, iter) => new BulkCsvReader(
+ file.mapPartitions { iter =>
--- End diff --
I see. Thanks! Then, should I close this or just get rid of comment changes here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [MINOR][SQL] Fix comments styl and correct sev...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12109#issuecomment-204358699
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [MINOR][SQL] Fix comments styl and correct sev...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12109#discussion_r58233581
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVRelation.scala ---
@@ -42,12 +42,12 @@ object CSVRelation extends Logging {
firstLine: String,
params: CSVOptions): RDD[Array[String]] = {
// If header is set, make sure firstLine is materialized before sending to executors.
- file.mapPartitionsWithIndex({
- case (split, iter) => new BulkCsvReader(
+ file.mapPartitions { iter =>
--- End diff --
This is legitimately a little optimization, but the rest feels too minor to bother with. Really there are more instances of slightly mis-spaced scaladoc or lines of code, though to fix them all would be time-consuming. Fixing them when nearby code changes is sensible. Fixing a few at random doesn't feel like a great goal.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [MINOR][SQL] Fix comments styl and correct sev...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12109
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [MINOR][SQL] Fix comments styl and correct sev...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12109#issuecomment-204358702
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54698/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org