You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ericjperry <gi...@git.apache.org> on 2017/10/19 15:03:37 UTC
[GitHub] spark pull request #19537: [SQL] Mark strategies with override for clarity.
GitHub user ericjperry opened a pull request:
https://github.com/apache/spark/pull/19537
[SQL] Mark strategies with override for clarity.
## What changes were proposed in this pull request?
This is a very trivial PR, simply marking `strategies` in `SparkPlanner` with the `override` keyword for clarity since it is overriding `strategies` in `QueryPlanner` two levels up in the class hierarchy. I was reading through the code to learn a bit and got stuck on this fact for a little while, so I figured this may be helpful so that another developer new to the project doesn't get stuck where I was.
I did not make a JIRA ticket for this because it is so trivial, but I'm happy to do so to adhere to the contribution guidelines if required.
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
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/ericjperry/spark override-strategies
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19537.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 #19537
----
commit 69f041f1420be262006163bb64c39daa205e7209
Author: Eric Perry <er...@ericjperry.com>
Date: 2017-10-19T13:05:16Z
Mark strategies as override for clarity.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19537: [SQL] Mark strategies with override for clarity.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19537
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19537: [SQL] Mark strategies with override for clarity.
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19537
**[Test build #3953 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3953/testReport)** for PR 19537 at commit [`69f041f`](https://github.com/apache/spark/commit/69f041f1420be262006163bb64c39daa205e7209).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19537: [SQL] Mark strategies with override for clarity.
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19537
Thanks! Merged to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19537: [SQL] Mark strategies with override for clarity.
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19537
**[Test build #3953 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3953/testReport)** for PR 19537 at commit [`69f041f`](https://github.com/apache/spark/commit/69f041f1420be262006163bb64c39daa205e7209).
* 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 #19537: [SQL] Mark strategies with override for clarity.
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19537
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19537: [SQL] Mark strategies with override for clarity.
Posted by ericjperry <gi...@git.apache.org>.
Github user ericjperry commented on the issue:
https://github.com/apache/spark/pull/19537
@srowen not that I noticed during my reading at least. I can't speak much about classes outside of the SQL portion of the project, though.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19537: [SQL] Mark strategies with override for clarity.
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/19537
I think that's OK. Are there other instances where something could `override` but doesn't?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org