-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split up codegen library #465
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all I understand, this seems like a good change and should indeed be transparent to clients. Looking forward to seeing the sbt-less build :-)
|
||
object ServerClientCodeGenerator extends CodeGenApp { | ||
trait ServerClientCodeGenerator extends CodeGenApp { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the naming could be simplified, since the package location already defines much of the semantics.
I'd suggest to name/move this to com.soundcloud.twinagle.CodeGenerator
. If it's a class instead of a trait it could be used without the need of an special Standalone*
object.
Consequently, the sbt extension could then be moved to com.soundcloud.twinagle.plugin.CodeGenerator
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CodeGenerator defines the main method though (via inheriting the CodeGenApp trait). Doesn't it need to be on a Scala object
for it to be statically callable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does the trait need to extend CodeGenApp
? I wonder if you could have something along the lines of
trait ServerClientCodeGenerator {
// ...lots of definitions
}
object StandaloneCodeGen extends CodeGenApp with ServerClientCodeGenerator {}
object SbtCodeGen extends CodeGenApp with ServerClientCodeGenerator {
override def suggestedDependencies = ...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't work unfortunately, because I need the override def
for registerExtensions
. When I try that, I get:
[error] object StandaloneServerClientCodeGenerator inherits conflicting members:
[error] method registerExtensions in trait CodeGenApp of type (registry: com.google.protobuf.ExtensionRegistry)Unit an
[error] method registerExtensions in trait ServerClientCodeGenerator of type (registry: com.google.protobuf.ExtensionRegistry)Unit
[error] (Note: this can be resolved by declaring an override in object StandaloneServerClientCodeGenerator.)
[error] object StandaloneServerClientCodeGenerator extends CodeGenApp with ServerClientCodeGenerator {}
[error] ^
[error] one error found
nice! this is something I'd considered a long time, since scalapb supports being run as a protoc extension but the integration with sbt made using that impossible 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
codegen/src/main/scala/com/soundcloud/twinagle/codegen/ServerClientCodeGenerator.scala
Show resolved
Hide resolved
1) a codegen library that can be used as a protoc plugin without SBT 2) an SBT compiler plugin that registers the plugin from (1)
I would like to use the Twinagle code generator in a Scala project without SBT. In my case, I'll be integrating it with Bazel using a custom scala_proto toolchain from rules_scala.
The code generator is only available as an SBT compiler plugin which presents two problems (1) the library is pegged at Scala 2.12, and (2) it cannot be used without SBT. So, for example, I cannot and do not wish to depend on the BuildInfo plugin.
The approach taken by this PR is to introduce a new Scala library
twinagle-codegen
that includes the code generator without any dependencies on SBT and adds this new library as a dependency to the existingtwinagle-scalapb-plugin
library. I believe this should be transparent to users sincetwinagle-codegen
will be pulled in as a transient dependency.