lihaoyi / autowire

Macros for simple/safe RPCs between Scala applications, including ScalaJS/ScalaJVM
378 stars 48 forks source link

Optimise materialised code + allow `= ???` and accessor methods #87

Closed marcgrue closed 4 years ago

marcgrue commented 4 years ago

Sorry for the large code base to review! I saw that @mathieuleclaire also just made the tests pass.

This pull request consists of basically 5 parts:

Allowing a shared interface to define methods with an undefined implementation (def someMethod() = ???) allows client code to extend such interfaces without having to implement the methods or being abstract itself.

Allowing accessor methods solves #36

(embarrassing fact: this is my first pull request - have been working solo on my own projects for years. So please let me know if I'm doing something wrong here)

mathieuleclaire commented 4 years ago

Thanks Marc for this (huge first) PR. Indeed we solved test in the same time :). I tried tests with your PR, they all pass.

Do you have any idea on how fix the Case Class test (https://github.com/marcgrue/autowire/blob/master/autowire/shared/src/test/scala/autowire/UpickleTests.scala#L125) ?

@lihaoyi, do you have any comment on other items ?

marcgrue commented 4 years ago

Thanks yourself, Mathieu, for putting time into Lihaoyi's excellent library!

I had a look at the Point case class test, yes, but I didn't manage to solve it, unfortunately. I suspect that the error:

Error:(19, 57) could not find implicit value for evidence parameter of type 
upickle.default.Reader[autowire.Point]
def routes: Bundle.Server.Router = Server.route[Api](Controller)

has something to do with upickle but I'm not sure :-(

mathieuleclaire commented 4 years ago

Yes definitely. I tried to implement something like this https://www.lihaoyi.com/upickle/#ManualSealedTraitPicklers, but It didn't work. I am now use Boopickle, so I do not really understand upickle nuances :) !

marcgrue commented 4 years ago

I'm happy it worked. Thanks, Mathieu!