Generate FinagledClient and FinagledService for java-experimental #138
Conversation
Support was probably broken in commit cad09af when the *Client and *Service classes were moved out of the "main" service file. This change was reflected in the generated Scala code, but not in the Java code. See also: https://groups.google.com/forum/#!topic/finaglers/W4hhEcAbocI - Configure compiler to output generated *Client and *Service classes to file - Rewrite relevant Java generators so that they generate code compatible with a class living in its own file (import statements, remove "static" keyword etc) - The "main" service class has a FinagledClient which extends (...ServiceName...)$FinagleClient, like in the Scala generated code.
| @@ -1,11 +1,46 @@ | |||
| package {{package}}; | |||
|
|
|||
| import com.twitter.scrooge.Option; | |||
There was a problem hiding this comment.
what happened here? why did we need so many new imports?
There was a problem hiding this comment.
There was a refactoring in cad09af that separated (in the Scala code) the Iface+FutureIface from the FinagledClient, so the FinagledClient was generated into its own file (instead of as an inner class).
So, what used to be just a template for a part of a class (finagleClient.scala and finagleService.scala) is now generating a file of its own, and required new imports, like the ones here:
https://github.com/eirslett/scrooge/commit/cad09af7963f01a3efbb8d5a5add8e7354bda1aa#diff-4
This is the same change, but for Java. Since we generate a new Java file, we need to import all the stuff again.
There was a problem hiding this comment.
(Just to clarify; the imports come from service.java, because that was where FinagledClient used to live)
| @@ -48,6 +48,27 @@ | |||
| {{>function}}; | |||
| {{/asyncFunctions}} | |||
| } | |||
|
|
|||
| class FinagledClient extends {{ServiceName}}$FinagleClient { | |||
There was a problem hiding this comment.
Should this (and FinagledService) be public? Without that I can't use them instead of {{ServiceName}}$FinagleService.
There was a problem hiding this comment.
You're right. They should probably all be static too. Fixed in 3a6a3b8
|
Can you merge master into this please? The Travis CI has been fixed recently. |
| import com.twitter.finagle.Service; | ||
| import com.twitter.finagle.SourcedException; | ||
| import com.twitter.finagle.stats.Counter; | ||
| import com.twitter.finagle.stats.NullStatsReceiver; |
There was a problem hiding this comment.
This is not used; cleanup unused imports please.
There was a problem hiding this comment.
They are all in use actually... unless I've missed something...
Service is used in finagleClient.java line 41 (using the full com.twitter... namespace though)
SourcedException used in finagleClient.java, line 89
Counter used in line 109
Service is used in line 41, but is fully namespaced, so we don't need to import it.
NullStatsReceiver is not used at all. I'll remove these two.
|
I've pulled this internally, and it should show up on master soon. Thanks for the patch! |
Support was probably broken in commit cad09af when
the *Client and *Service classes were moved out of the "main"
service file. This change was reflected in the generated Scala code,
but not in the Java code.
See also: https://groups.google.com/forum/#!topic/finaglers/W4hhEcAbocI
to file
compatible with a class living in its own file
(import statements, remove "static" keyword etc)
(...ServiceName...)$FinagleClient, like in the Scala generated code.