Feature/retrofit2 rx

[kungfoo]

It should be able to use the RxJava flavor of retrofit, so I added a generator option for that.

[wing328]

@kungfoo thanks for the contribution. Would it be better to keep just one Retrofit2 template and add the support of RxJava via a CLI option (e.g. supportRxJava) ?

[kungfoo]

I thought about that, but then again, the dependencies might be quite
different in the gradle files, so there might actually be a lot of
conditional logic in the mustache files. I can however try to migrate to
using an option today... I simply had to get the thing going first. :)

On Thu, Jan 21, 2016 at 4:52 AM, wing328 [email protected] wrote:

@kungfoo https://github.com/kungfoo thanks for the contribution. Would
it be better to keep just one Retrofit2 template and add the support of
RxJava via a CLI option (e.g. supportRxJava) ?

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

[kungfoo]

Can I add options for the library to config.json or does it have to be a CLI option?

[wing328]

I think it works both way.If you look at the library option, it can be specified in config.json or via -Dlibrary=retrofit

[kungfoo]

Okay, I will try adding it as an option -DuseRxJava=true, which only makes sense (currently) when using the retrofit2 library.

[kungfoo]

I updated the PR to change the existing templates rather than create new
ones. If you can take a look, that would be most awesome.

On Thu, Jan 21, 2016 at 11:01 AM, wing328 [email protected] wrote:

I think it works both way.If you look at the library option, it can be
specified in config.json or via -Dlibrary=retrofit

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

[wing328]

One suggestion is to create a ./bin/java-petstore-retrofit2rx.sh based on ./bin/java-petstore-retrofit2.sh and generate the sample Retrofit2 RX under samples/client/petstore/java/retrofit2rx

Then copy the unit test from retrofit2 to retrofit2rx to see if all tests pass

[kungfoo]

Will do, I will have to rewrite the tests to use Observable. I can use java-8 in the sample, right?

[wing328]

My understanding is that both Retrofit2 and RxJava support Java7 so I would suggest using Java7 for now.

[kungfoo]

As far as I know, both libraries support java-8, RxJava is even more fun to
use with the new features.
Since it is a sample that should show what the library is capable of, I
will try to make it short and succinct. Of course the tests have to pass.

On Fri, Jan 22, 2016 at 9:51 AM, wing328 [email protected] wrote:

My understanding is that both Retrofit2 and RxJava support Java7 so I
would suggest using Java7 for now.

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

[kungfoo]

Or do you regenerate the samples on every build?

[wing328]

@kungfoo you've to generate the samples manually by running the corresponding shell script under ./bin/

[kungfoo]

Which is what I did. I still had to adjust the tests to use the API provided with the RxJavaAdapter of retrofit.

[wing328]

If you're referring to the CI tests, which is triggered by mvn -verify -P samples and in order for your newly generated code to be covered by mvn -verify -P samples, you will need to update pom.xml.

I would recommend updating pom.xml to include the retrofit2rx later (as a day 2 requirement)

[kungfoo]

Hmm, the tests now fail on 1.7, I see. I did add samples that are 1.8 under samples/retrofit2rx. So I will have to rewrite the tests using 1.7 features (anonymous classes agogo, yay.).

Actually: The generated client code could actually be 1.7 as long as the tests that are executed can be 1.8, which is a problem on Travis, as far as I can tell. Bummer.

[kungfoo]

I did add the samples and tests and rewrote them using java 7, so now they
are executed and run on both tested vms.

On Fri, 22 Jan 2016, 15:23 wing328 [email protected] wrote:

If you're referring to the CI tests, which is triggered by mvn -verify -P
samples and in order for your newly generated code to be covered by mvn
-verify -P samples, you will need to update pom.xml.

I would recommend updating pom.xml to include the retrofit2rx later (as a
day 2 requirement)

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

[wing328]

@kungfoo thanks! Do you mind pulling the latest master and rebase on that to resolve the merge conflicts?

[kungfoo]

Nope, will do asap.
On 25 Jan 2016 04:13, "wing328" [email protected] wrote:

@kungfoo https://github.com/kungfoo thanks! Do you mind pulling the
latest master and rebase on that to resolve the merge conflicts?

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

[wing328]

I did some tests and the result looks good. PR merged