[WIP][SPARK-14629][SPARK-14630][Build] Add support for custom scala style rules & Add rule PublicAbstractMethodsHaveTypeChecker #12396
Conversation
|
Test build #55821 has finished for PR 12396 at commit
|
|
As per Scalastyle - Custom rules, I installed the jar on my local machine, and verified that the added rule works. Now I'm not sure how to pre-install the jar for every Spark build on Jenkins's machine -- should some build scripts be modified? @rxin could you give some hints? Thank you! :-) |
lw-lin
changed the title
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.apache.spark</groupId> | ||
| <artifactId>scala_style_checker</artifactId> |
There was a problem hiding this comment.
Yikes, do we have to maintain and publish this as a module to use it? it may not be worth it
There was a problem hiding this comment.
@srowen yeah I think you're right -- doing that is heavy and not worth it. :-)
I'm working on a light-weight manner to do this, after all all we need is to install the custom rules jar locally in the dev & build environment, other than publishing that into run-time environment or maven-central.
Thanks for the review!
|
Test build #55823 has finished for PR 12396 at commit
|
…t methods should have explicit return types ## What changes were proposed in this pull request? Currently many public abstract methods (in abstract classes as well as traits) don't declare return types explicitly, such as in [o.a.s.streaming.dstream.InputDStream](https://github.com/apache/spark/blob/master/streaming/src/main/scala/org/apache/spark/streaming/dstream/InputDStream.scala#L110): ```scala def start() // should be: def start(): Unit def stop() // should be: def stop(): Unit ``` These methods exist in core, sql, streaming; this PR fixes them. ## How was this patch tested? N/A ## Which piece of scala style rule led to the changes? the rule was added separately in #12396 Author: Liwei Lin <[email protected]> Closes #12389 from lw-lin/public-abstract-methods.
|
hi @vanzin (with some custom rules jar being built and pre-installed, we then can enable custom code style checks, please see http://www.scalastyle.org/custom-rules.html) |
|
we can push this into scalastyle and then upgrade the version. |
|
@lw-lin I don't know either, but that sounds like too much work for style rules. I suggest following Reynold's advice and posting a PR to the scalastyle project; they're a little slow to update things, though. |
lw-lin
changed the title
lw-lin
changed the title
5 participants
What changes were proposed in this pull request?
PublicAbstractMethodsHaveTypeCheckerimplementation, which would ensure public abstract methods have explicit return typesHow was this patch tested?
the list of changes-to-make found by this
PublicAbstractMethodsHaveTypeCheckerrule is here: https://github.com/apache/spark/pull/12389/files