[SPARK-7727] Avoid inner classes in RuleExecutor

[Jeffrharr]

Allow composibility of Batch in RuleExecutor be making classes outer.

[chenghao-intel]

Any background why we should expose the Batch? Per my understanding, we'd better keep the minimizing the class visibility as much as possible in design.

[Jeffrharr]

In my opinion, it seems like a poor fix to the issue of composing Batches
in different instances and there is likely a much better way to refactor
the class to get the desired behavior (although I don't personally know how
yet). It was labeled as starter and making the classes outer was the
proposed fix.
On May 24, 2015 11:48 PM, "Cheng Hao" notifications@github.com wrote:

Any background why we should expose the Batch? Per my understanding, we'd
better keep the minimizing the class visibility as much as possible in
design.

—
Reply to this email directly or view it on GitHub
#6319 (comment).

[chenghao-intel]

Oh, sorry, I didn't see the description of the jira. I will jump there to give more comments.

[evacchi]

@chenghao-intel although Batch may not be supposed to be public, inner classes are still bad when it comes to sharing code (because they are not static inner). I'd advise to make them at least inner to a companion object; see PR #6349

[rxin]

Can you add the explicit type back?

[rxin]

LGTM other than that explicit type.

[rxin]

Jenkins, test this please.

[SparkQA]

Test build #33816 has finished for PR 6319 at commit ce264cb.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class Batch[TreeType <: TreeNode[_]](name: String, strategy: Strategy, rules: Rule[TreeType]*)
  • abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging

[marmbrus]

ping, mind bringing this up to date?

[rxin]

Jenkins, ok to test.

[marmbrus]

ok to test

[SparkQA]

Test build #1174 has finished for PR 6319 at commit 0c38212.

• This patch fails to build.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class Batch[TreeType <: TreeNode[_]](name: String, strategy: Strategy, rules: Rule[TreeType]*)
  • abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging

[marmbrus]

@His-name-is-Joof, thanks for working on this. I propose we close this issue until you have time to fix the build failures and bring the code up to date.