You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by wangyum <gi...@git.apache.org> on 2018/09/27 13:18:43 UTC
[GitHub] spark pull request #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringC...
GitHub user wangyum opened a pull request:
https://github.com/apache/spark/pull/22570
[SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker to scalastyle config
## What changes were proposed in this pull request?
[EmptyInterpolatedStringChecker](http://www.scalastyle.org/rules-dev.html#org_scalastyle_scalariform_EmptyInterpolatedStringChecker) used for check for empty string interpolations. This feature is very useful to us. This PR add it to `scalastyle-config.xml` and fix all empty interpolated string issue.
## How was this patch tested?
manual tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/wangyum/spark SPARK-25553
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22570.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 #22570
----
commit 0a01aa311f038771706cb029cf89607390f0cd57
Author: Yuming Wang <yu...@...>
Date: 2018-09-27T12:43:15Z
Add EmptyInterpolatedStringChecker to scalastyle-config.xml
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96698/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22570
**[Test build #96698 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96698/testReport)** for PR 22570 at commit [`68f83a3`](https://github.com/apache/spark/commit/68f83a366497c1b263d4f4bf67ad46bcc5c65c6d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3537/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringC...
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum closed the pull request at:
https://github.com/apache/spark/pull/22570
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22570
For me, this PR is too intrusive to merge at this stage.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on the issue:
https://github.com/apache/spark/pull/22570
cc @dongjoon-hyun @HyukjinKwon @srowen
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3540/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3538/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96694/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringC...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22570#discussion_r220933694
--- Diff: core/src/main/scala/org/apache/spark/deploy/rest/StandaloneRestServer.scala ---
@@ -91,7 +91,7 @@ private[rest] class StandaloneStatusRequestServlet(masterEndpoint: RpcEndpointRe
protected def handleStatus(submissionId: String): SubmissionStatusResponse = {
val response = masterEndpoint.askSync[DeployMessages.DriverStatusResponse](
DeployMessages.RequestDriverStatus(submissionId))
- val message = response.exception.map { s"Exception from the cluster:\n" + formatException(_) }
+ val message = response.exception.map { "Exception from the cluster:\n" + formatException(_) }
--- End diff --
In cases like this, we should use interpolation in place of concatenation, if anything. Likewise the previous instance, for example.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22570
**[Test build #96696 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96696/testReport)** for PR 22570 at commit [`4adfa46`](https://github.com/apache/spark/commit/4adfa46dfda68ede43f68485e6df36b50b8dfa96).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22570
**[Test build #96698 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96698/testReport)** for PR 22570 at commit [`68f83a3`](https://github.com/apache/spark/commit/68f83a366497c1b263d4f4bf67ad46bcc5c65c6d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22570
**[Test build #96694 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96694/testReport)** for PR 22570 at commit [`0a01aa3`](https://github.com/apache/spark/commit/0a01aa311f038771706cb029cf89607390f0cd57).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22570
We had better update the code when we touch it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22570
**[Test build #96694 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96694/testReport)** for PR 22570 at commit [`0a01aa3`](https://github.com/apache/spark/commit/0a01aa311f038771706cb029cf89607390f0cd57).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` println(\"Per-class example fractions, counts:\")`
* ` instr.logWarning(\"All labels belong to a single class and fitIntercept=false. It's a \" +`
* ` require(className == expectedClassName, \"Error loading metadata: Expected class name\" +`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on the issue:
https://github.com/apache/spark/pull/22570
What do we do next?
1. Change `level` to `warning`and commit all changes. In this case `dev/scalastyle` can pass, but IDEA will have a warning:
2. Remove `EmptyInterpolatedStringChecker` and commit all changes.
3. Just close this PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22570
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96696/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22570
**[Test build #96696 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96696/testReport)** for PR 22570 at commit [`4adfa46`](https://github.com/apache/spark/commit/4adfa46dfda68ede43f68485e6df36b50b8dfa96).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22570: [SPARK-25553][BUILD] Add EmptyInterpolatedStringChecker ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22570
Exactly same opinion with Sean's.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org