Github user rdblue commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164806676
--- Diff:
sql/core/src/main/java/org/apache/spark/sql/sources/v2/SessionConfigSupport.java
---
@@ -25,7 +25,7 @@
* session.
*/
@InterfaceStability.Evolving
-public interface SessionConfigSupport {
+public interface SessionConfigSupport extends DataSourceV2 {
--- End diff --
Mixing large migration commits like this one with unrelated changes makes
it harder to pick or revert changes without unintended side-effects. What
happens if we realize that this rename was a bad idea? Reverting this commit
would also revert the constraint that SessionConfigSupport extends
DataSourceV2. Similarly, if we realize that these mix-ins don't need to extend
DataSourceV2, then we would have to find and remove them all instead of
reverting a commit. That might even sound okay, but when you're picking commits
deliberately to patch branches, you need to make as few changes as possible and
cherry-pick conflicts make that much harder.
The fact that you're rushing to get commits into 2.3 is even more
concerning and reason to be careful, not a reason to relax our standards.
Please move this to its own PR and fix all of the interfaces at once.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
