Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/8111#issuecomment-130103149
Reviewed 28 of 131 files at r1, 1 of 2 files at r2.
Review status: 29 of 132 files reviewed at latest revision, 4 unresolved
discussions, all commit checks successful.
---
<sup>**[project/SparkBuild.scala, line 344
\[r4\]](https://reviewable.io:443/reviews/apache/spark/8111#-JwTp9kWQsaqCdeebIfC)**
([raw
file](https://github.com/apache/spark/blob/f16bc68dfb25c7b746ae031a57840ace9bafa87f/project/SparkBuild.scala#L344)):</sup>
This is probably safe to remove given that we should inherit this from the
global TestSettings.
---
<sup>**[project/SparkBuild.scala, line 363
\[r4\]](https://reviewable.io:443/reviews/apache/spark/8111#-JwTp7m_pjFebdyrnVpO)**
([raw
file](https://github.com/apache/spark/blob/f16bc68dfb25c7b746ae031a57840ace9bafa87f/project/SparkBuild.scala#L363)):</sup>
Should still have something here to create the context and import its
implicits. This will require manual testing before signoff.
---
<sup>**[sql/core/src/test/java/test/org/apache/spark/sql/JavaDataFrameSuite.java,
line 46
\[r4\]](https://reviewable.io:443/reviews/apache/spark/8111#-JwTqqTK9nRnSOw-OVst)**
([raw
file](https://github.com/apache/spark/blob/c92a3b04883cfca1c3e0b71048aba9471c9fde26/sql/core/src/test/java/test/org/apache/spark/sql/JavaDataFrameSuite.java#L46)):</sup>
Note that this is called on every test, not once per suite, so I think that
we'll wind up re-loading the test data many times. If this turns out to be a
perf. issue then we may want to use JUnit BeforeAll or BeforeSuite or whatever
it's called.
---
<sup>**[sql/core/src/test/scala/org/apache/spark/sql/test/SQLTestUtils.scala,
line 49
\[r4\]](https://reviewable.io:443/reviews/apache/spark/8111#-JwTxpbhNWhfdKVR8Jen)**
([raw
file](https://github.com/apache/spark/blob/c92a3b04883cfca1c3e0b71048aba9471c9fde26/sql/core/src/test/scala/org/apache/spark/sql/test/SQLTestUtils.scala#L49)):</sup>
As discussed offline, I think that SQLTestUtils should remain a trait that
is mixed into SharedSQLContext and SharedHiveContext. This will allow its
helper methods to be used in non-shared-context settings as well.
---
Comments from the [review on
Reviewable.io](https://reviewable.io:443/reviews/apache/spark/8111)
<!-- Sent from Reviewable.io -->
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
