Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MINOR] Revert removing explicit typing (changed in some examples and StatFunctions) #12452

Closed
wants to merge 1 commit into from
Closed

[MINOR] Revert removing explicit typing (changed in some examples and StatFunctions) #12452

wants to merge 1 commit into from

Conversation

HyukjinKwon
Copy link
Member

What changes were proposed in this pull request?

This PR reverts some changes in #12413. (please see the discussion in that PR).

from

    words.foreachRDD { (rdd, time) => 
    ...

to

    words.foreachRDD { (rdd: RDD[String], time: Time) =>
    ...

Also, this was discussed in dev-mailing list, here

How was this patch tested?

This was tested with sbt scalastyle.

@HyukjinKwon HyukjinKwon changed the title [MINOR] Revert removing explicit typing [MINOR] Revert removing explicit typing (changed in some examples and StatFunctions) Apr 17, 2016
@HyukjinKwon
Copy link
Member Author

cc @rxin

@rxin
Copy link
Contributor

rxin commented Apr 17, 2016

Thanks! LGTM.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Apr 17, 2016 •
edited
Loading

Actually, shouldn't we maybe a bit wait for arriving at conclusion? I am leaning toward that this change ( I meant reverting this) is correct but feel like some think not although it is a tiny minor change.

+As said here, I also agree to just merge this if you feel strongly this is correct.

@SparkQA
Copy link

SparkQA commented Apr 17, 2016

Test build #56046 has finished for PR 12452 at commit 52f1324.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

srowen
srowen reviewed Apr 17, 2016
View reviewed changes
examples/src/main/scala/org/apache/spark/examples/streaming/RecoverableNetworkWordCount.scala
@@ -116,7 +116,7 @@ object RecoverableNetworkWordCount {
val lines = ssc.socketTextStream(ip, port)
val words = lines.flatMap(_.split(" "))
val wordCounts = words.map((_, 1)).reduceByKey(_ + _)
wordCounts.foreachRDD { (rdd, time) =>
wordCounts.foreachRDD { (rdd: RDD[(String, Int)], time: Time) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I'm fine with putting this back, myself, but it's not consistent with all of the other examples -- streaming ones yes but not the non-streaming ones. Not a big deal. I think it's worth just getting this resolved.

@HyukjinKwon
Copy link
Member Author

retest this please

@SparkQA
Copy link

SparkQA commented Apr 17, 2016

Test build #56055 has finished for PR 12452 at commit 52f1324.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member Author

retest this please

@SparkQA
Copy link

SparkQA commented Apr 17, 2016

Test build #56060 has finished for PR 12452 at commit 52f1324.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member Author

retest this please

@HyukjinKwon
Copy link
Member Author

test this please

@HyukjinKwon
Copy link
Member Author

(The tests org.apache.spark.sql.hive.HiveSparkSubmitSuite.SPARK-8020: set sql conf in spark conf and
org.apache.spark.sql.hive.HiveSparkSubmitSuite.SPARK-9757 Persist Parquet relation with decimal column pass in my local)

@SparkQA
Copy link

SparkQA commented Apr 18, 2016

Test build #56076 has finished for PR 12452 at commit 52f1324.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 18, 2016

Test build #2814 has finished for PR 12452 at commit 52f1324.

  • This patch passes all tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented Apr 18, 2016

OK i'm just going to merge this. It's actually not that big of a deal, but I wanted to make a point about avoiding changing code for the sake of changing code. Thanks a lot for bearing with me @HyukjinKwon .

@asfgit asfgit closed this in 6fc1e72 Apr 18, 2016
lw-lin pushed a commit to lw-lin/spark that referenced this pull request Apr 20, 2016
@HyukjinKwon @lw-lin
[MINOR] Revert removing explicit typing (changed in some examples and…
effd507 
… StatFunctions)

## What changes were proposed in this pull request?

This PR reverts some changes in apache#12413. (please see the discussion in that PR).

from
```scala
    words.foreachRDD { (rdd, time) =>
    ...
```

to
```scala
    words.foreachRDD { (rdd: RDD[String], time: Time) =>
    ...
```

Also, this was discussed in dev-mailing list, [here](http://apache-spark-developers-list.1001551.n3.nabble.com/Question-about-Scala-style-explicit-typing-within-transformation-functions-and-anonymous-val-td17173.html)

## How was this patch tested?

This was tested with `sbt scalastyle`.

Author: hyukjinkwon <gurwls223@gmail.com>

Closes apache#12452 from HyukjinKwon/revert-explicit-typing.
@HyukjinKwon HyukjinKwon mentioned this pull request Jul 4, 2016
Closed
@HyukjinKwon HyukjinKwon deleted the revert-explicit-typing branch January 2, 2018 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@HyukjinKwon @rxin @SparkQA @srowen