[GitHub] incubator-toree pull request #151: [TOREE-463] break out scala interpreter i...

[sanjay-saxena <gi...@git.apache.org>]

Github user sanjay-saxena commented on a diff in the pull request:

    https://github.com/apache/incubator-toree/pull/151#discussion_r166772771
  
    --- Diff: kernel/src/main/scala/org/apache/toree/boot/layer/ComponentInitialization.scala ---
    @@ -28,6 +28,7 @@ import org.apache.toree.comm.{CommManager, CommRegistrar, CommStorage, KernelCom
     import org.apache.toree.dependencies.{CoursierDependencyDownloader, Credentials, DependencyDownloader}
     import org.apache.toree.interpreter._
     import org.apache.toree.kernel.api.Kernel
    +import org.apache.toree.kernel.interpreter.scala.ScalaInterpreter
    --- End diff --
    
    Based on my limited understanding of the code, picking up a dependency on an implementation such as `ScalaInterpreter` from `scala-interpreter` project in the generic `kernel` project maybe an abstraction violation. It may also lead to circular dependencies. Maybe, you should look at either reusing the existing APIs on the generic trait `Interpreter`. And, if the existing APIs are not sufficient, then maybe add new APIs to the generic trait `Interpreter`.


---