You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gengliangwang <gi...@git.apache.org> on 2017/06/29 23:49:22 UTC
[GitHub] spark pull request #18471: [SPARK-21259] More rules for scalastyle
GitHub user gengliangwang opened a pull request:
https://github.com/apache/spark/pull/18471
[SPARK-21259] More rules for scalastyle
## What changes were proposed in this pull request?
Add more rules for scalastyle
During code review, we spent so much time on code style issues.
It would be great if we add rules:
1) disallow space before colon
2) disallow space before right parentheses
3) disallow space after left parentheses
## How was this patch tested?
unit tests, integration tests
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gengliangwang/spark scalastyle
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18471.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 #18471
----
commit 3a4eee0a83de1f3830c101a1dfdfa340dba15758
Author: Wang Gengliang <lt...@gmail.com>
Date: 2017-06-29T22:01:17Z
first run
commit ff6a50a17d9a77049be7642e66c400d9fab87375
Author: Wang Gengliang <lt...@gmail.com>
Date: 2017-06-29T23:20:57Z
resolve hard ones
commit 6eccd2636e577439d3539eea79068abadd6122dc
Author: Wang Gengliang <lt...@gmail.com>
Date: 2017-06-29T23:24:47Z
fix one error
commit cb602758437a2cf7752547433de1a85bd36b0037
Author: Wang Gengliang <lt...@gmail.com>
Date: 2017-06-29T23:28:09Z
resolve more
commit 6329dfb1304f5a1ff3c210a464b1f11b9b3b9760
Author: Wang Gengliang <lt...@gmail.com>
Date: 2017-06-29T23:34:53Z
more scalastyle off/on
commit f68e60014ca2f1dbb502dbb942e109505c892c5a
Author: Wang Gengliang <lt...@gmail.com>
Date: 2017-06-29T23:36:48Z
Insert a space after the start of the comment
----
---
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 #18471: [SPARK-21259] More rules for scalastyle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r125026821
--- Diff: core/src/main/scala/org/apache/spark/Accumulable.scala ---
@@ -68,7 +68,9 @@ class Accumulable[R, T] private (
}
private[spark] def this(initialValue: R, param: AccumulableParam[R, T], name: Option[String]) = {
+ // scalastyle:off
this(initialValue, param, name, false /* countFailedValues */)
+ // scalastyle:on
--- End diff --
I believe we could enable and disable via `scalastyle:on customid` rather than switching off whole style rules here IIRC.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18471
Build finished. Test FAILed.
---
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 #18471: [SPARK-21259] More rules for scalastyle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r125022780
--- Diff: core/src/main/scala/org/apache/spark/broadcast/BroadcastManager.scala ---
@@ -52,8 +52,8 @@ private[spark] class BroadcastManager(
private val nextBroadcastId = new AtomicLong(0)
- def newBroadcast[T: ClassTag](value_ : T, isLocal: Boolean): Broadcast[T] = {
- broadcastFactory.newBroadcast[T](value_, isLocal, nextBroadcastId.getAndIncrement())
+ def newBroadcast[T: ClassTag](_value: T, isLocal: Boolean): Broadcast[T] = {
--- End diff --
I am not sure of this naming change. I mean the change does not look particulaily better to me. I believe we have a open PR - https://github.com/databricks/scala-style-guide/pull/52 which appearntly stays against some cases 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 #18471: [SPARK-21259] More rules for scalastyle
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang closed the pull request at:
https://github.com/apache/spark/pull/18471
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:
https://github.com/apache/spark/pull/18471
Do we still want this?
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18471
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78940/
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 #18471: [SPARK-21259] More rules for scalastyle
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r125029078
--- Diff: core/src/main/scala/org/apache/spark/broadcast/BroadcastManager.scala ---
@@ -52,8 +52,8 @@ private[spark] class BroadcastManager(
private val nextBroadcastId = new AtomicLong(0)
- def newBroadcast[T: ClassTag](value_ : T, isLocal: Boolean): Broadcast[T] = {
- broadcastFactory.newBroadcast[T](value_, isLocal, nextBroadcastId.getAndIncrement())
+ def newBroadcast[T: ClassTag](_value: T, isLocal: Boolean): Broadcast[T] = {
--- End diff --
I think it's not about adding `_` to the name of private val, but kind of avoiding name conflict if 2 variables have similar meaning. At least this is fine to me, I also do this a lot...
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18471
**[Test build #78940 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78940/testReport)** for PR 18471 at commit [`d1b4bbe`](https://github.com/apache/spark/commit/d1b4bbe46a279959816e57a9c16636dd5e2d23ad).
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18471
**[Test build #78951 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78951/testReport)** for PR 18471 at commit [`fd5bde5`](https://github.com/apache/spark/commit/fd5bde5e1c79c68bebd667828d773f63165117cb).
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18471
Merged build finished. Test FAILed.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18471
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78937/
Test FAILed.
---
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 #18471: [SPARK-21259] More rules for scalastyle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r125032760
--- Diff: core/src/main/scala/org/apache/spark/broadcast/BroadcastManager.scala ---
@@ -52,8 +52,8 @@ private[spark] class BroadcastManager(
private val nextBroadcastId = new AtomicLong(0)
- def newBroadcast[T: ClassTag](value_ : T, isLocal: Boolean): Broadcast[T] = {
- broadcastFactory.newBroadcast[T](value_, isLocal, nextBroadcastId.getAndIncrement())
+ def newBroadcast[T: ClassTag](_value: T, isLocal: Boolean): Broadcast[T] = {
--- End diff --
Maybe, this can be an example other contributors like me can refer in the future BTW.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18471
My only concern is `[K : ClassTag]`. This is conventional in scala but the new style check rule rejects it. If it's hard to make a corrected rule for this case, we can drop it.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:
https://github.com/apache/spark/pull/18471
Most changes are done by my ruby script:
```ruby
lines = `./dev/lint-scala 2>&1 | grep "before\\|after"`.split("\n")
lines.each do |line|
file, line_num=line.split(/[ ,:]/)[1,2]
if line =~ /Space before token :/
`sed -i '' '#{line_num}s/ :/:/g' #{file}`
elsif line =~ /Space before token \)/
`sed -i '' '#{line_num}s/ )/)/g' #{file}`
elsif line =~ /Space after token \(/
`sed -i '' '#{line_num}s/( /(/g' #{file}`
end
end
```
---
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 #18471: [SPARK-21259] More rules for scalastyle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r125032238
--- Diff: core/src/main/scala/org/apache/spark/broadcast/BroadcastManager.scala ---
@@ -52,8 +52,8 @@ private[spark] class BroadcastManager(
private val nextBroadcastId = new AtomicLong(0)
- def newBroadcast[T: ClassTag](value_ : T, isLocal: Boolean): Broadcast[T] = {
- broadcastFactory.newBroadcast[T](value_, isLocal, nextBroadcastId.getAndIncrement())
+ def newBroadcast[T: ClassTag](_value: T, isLocal: Boolean): Broadcast[T] = {
--- End diff --
Yea, not this one but some cases look provate ones. I am fine if we prefer a specific way. My only worry is we happen to fix it back again in the future but no one stays against because they don't know which way is preferred...
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18471
Merged build finished. Test FAILed.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18471
**[Test build #78951 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78951/testReport)** for PR 18471 at commit [`fd5bde5`](https://github.com/apache/spark/commit/fd5bde5e1c79c68bebd667828d773f63165117cb).
* This patch **fails due to an unknown error code, -9**.
* 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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18471
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 #18471: [SPARK-21259] More rules for scalastyle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r124948812
--- Diff: scalastyle-config.xml ---
@@ -46,7 +46,7 @@ This file is divided into 3 sections:
<check level="error" class="org.scalastyle.file.FileTabChecker" enabled="true"></check>
- <check level="error" class="org.scalastyle.file.HeaderMatchesChecker" enabled="true">
+ <check level="error" class="org.scalastyle.file.HeaderMatchesChecker" enabled="false">
--- End diff --
Just a question. Should we disable this rule to enable the new rule?
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18471
**[Test build #78948 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78948/testReport)** for PR 18471 at commit [`05084a9`](https://github.com/apache/spark/commit/05084a9706b2c3a9cf6dc1c733d7d16b8bac0f41).
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18471
**[Test build #78940 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78940/testReport)** for PR 18471 at commit [`d1b4bbe`](https://github.com/apache/spark/commit/d1b4bbe46a279959816e57a9c16636dd5e2d23ad).
* 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 #18471: [SPARK-21259] More rules for scalastyle
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r124964773
--- Diff: scalastyle-config.xml ---
@@ -245,7 +245,13 @@ This file is divided into 3 sections:
<check level="error" class="org.scalastyle.scalariform.DisallowSpaceBeforeTokenChecker" enabled="true">
<parameters>
- <parameter name="tokens">COMMA</parameter>
+ <parameter name="tokens">COLON, COMMA, RPAREN</parameter>
+ </parameters>
+ </check>
+
+ <check level="error" class="org.scalastyle.scalariform.DisallowSpaceAfterTokenChecker" enabled="true">
--- End diff --
Thanks, I have updated the xml file.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:
https://github.com/apache/spark/pull/18471
Close this for now.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/18471
I tend to agree with the style change, but, am concerned about what a huge diff this creates for little gain. I personally am not sure I'd bother with the colon change; some instances of space before colon are conventional like `[K : ClassTag]`, although many you fixed should indeed not have had a space. I also am not sure we want to enforce the naming convention you suggest here.
The space inside parens change seems OK, maybe smaller.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18471
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78951/
Test FAILed.
---
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 #18471: [SPARK-21259] More rules for scalastyle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r124949026
--- Diff: scalastyle-config.xml ---
@@ -245,7 +245,13 @@ This file is divided into 3 sections:
<check level="error" class="org.scalastyle.scalariform.DisallowSpaceBeforeTokenChecker" enabled="true">
<parameters>
- <parameter name="tokens">COMMA</parameter>
+ <parameter name="tokens">COLON, COMMA, RPAREN</parameter>
+ </parameters>
+ </check>
+
+ <check level="error" class="org.scalastyle.scalariform.DisallowSpaceAfterTokenChecker" enabled="true">
--- End diff --
Another question. Could we add `customId` and disable this only when `scalastyle:off` and `scalastyle:on` is required?
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18471
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78948/
Test FAILed.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18471
**[Test build #78937 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78937/testReport)** for PR 18471 at commit [`f68e600`](https://github.com/apache/spark/commit/f68e60014ca2f1dbb502dbb942e109505c892c5a).
* This patch **fails Spark unit tests**.
* This patch **does not merge 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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18471
**[Test build #78948 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78948/testReport)** for PR 18471 at commit [`05084a9`](https://github.com/apache/spark/commit/05084a9706b2c3a9cf6dc1c733d7d16b8bac0f41).
* This patch **fails due to an unknown error code, -9**.
* 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 #18471: [SPARK-21259] More rules for scalastyle
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r124941259
--- Diff: core/src/main/scala/org/apache/spark/Accumulable.scala ---
@@ -68,7 +68,9 @@ class Accumulable[R, T] private (
}
private[spark] def this(initialValue: R, param: AccumulableParam[R, T], name: Option[String]) = {
+ // scalastyle:off
this(initialValue, param, name, false /* countFailedValues */)
--- End diff --
nit: `countFailedValues = false` is better, this style is only useful for java
---
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 #18471: [SPARK-21259] More rules for scalastyle
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r124952579
--- Diff: scalastyle-config.xml ---
@@ -46,7 +46,7 @@ This file is divided into 3 sections:
<check level="error" class="org.scalastyle.file.FileTabChecker" enabled="true"></check>
- <check level="error" class="org.scalastyle.file.HeaderMatchesChecker" enabled="true">
+ <check level="error" class="org.scalastyle.file.HeaderMatchesChecker" enabled="false">
--- End diff --
No, I made a mistake.
---
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 issue #18471: [SPARK-21259] More rules for scalastyle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18471
**[Test build #78937 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78937/testReport)** for PR 18471 at commit [`f68e600`](https://github.com/apache/spark/commit/f68e60014ca2f1dbb502dbb942e109505c892c5a).
---
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 #18471: [SPARK-21259] More rules for scalastyle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18471#discussion_r125027002
--- Diff: core/src/main/scala/org/apache/spark/Accumulable.scala ---
@@ -68,7 +68,9 @@ class Accumulable[R, T] private (
}
private[spark] def this(initialValue: R, param: AccumulableParam[R, T], name: Option[String]) = {
+ // scalastyle:off
this(initialValue, param, name, false /* countFailedValues */)
+ // scalastyle:on
--- End diff --
Oh, nvm. I just read your commit log ...
---
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