Skip to content
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

RxGroovy for RxJava 1.1.X #13

Open
maltalex opened this issue Feb 14, 2016 · 4 comments
Open

RxGroovy for RxJava 1.1.X #13

maltalex opened this issue Feb 14, 2016 · 4 comments

Comments

@maltalex
Copy link

Are there plans to release RxGroovy for 1.1.0/1.1.1 ?
It's is currently stuck at 1.0.15 from October 15.

@sfitts
Copy link

sfitts commented Jun 7, 2016

Continuing a thread from ReactiveX/RxJava#3998

@akarnokd - do you need someone to help maintain this? While parallel releases are obviously possible, it is a lot nicer when everything is clean. Espeically given the usage of these in upstream projects like Vertx -- it will be difficult, if not impossible for me to convince that project to use a forked release for their dependencies. Anyway, just a thought. A quick review/build of the code seems to show that is "just works", though there is a bit about a custom create signature that might be able to be removed at this point (though I don't think there is any harm in keeping it).

@sfitts
Copy link

sfitts commented Jun 7, 2016

FWIW, since Groovy 2.2.x most of what is done with this library isn't needed anymore (at least AFAICT). Groovy supports implicit coversion of closures to SAM types and that works transparently with all cases here except for FuncN. In that case the issue is that groovy treats the args as an array and can't figure out that it can splat them into the varargs. So in that case we will need the extra meta-method to do the work.

@akarnokd
Copy link
Member

akarnokd commented Jun 7, 2016

I don't know Groovy much so can't really help with coding in this project. I assume bumping to RxJava 1.1.5 is a straightforward diff though.

@sfitts
Copy link

sfitts commented Jun 7, 2016

I don't profess to be a Groovy internals expert, but I know enough to be dangerous. This code basically uses Groovy meta-programming to dynamically override specific methods so it can convert Groovy closures into the various functional interfaces used in RxJava.

I think there are 2 approaches here. The first is to do as you suggest and just bump the RxJava dependency to 1.1.+. This is trivial and works fine. However, it does mean that you are doing some work that isn't really needed (with at least some runtime cost).

You can remove this by narrowing the cases where you override the methods. On Groovy 2.4.x (and I think earlier versions as well) the only case that isn't handled properly is the conversion from a closure to the interface FuncN. Here we still need the overriden meta-method to execute the closure properly. So if you don't mind supporting only more modern versions of Groovy (2.4 goes back to Jan 15, 2.2 to Nov 13) then you can simplify this quite a bit.

Let me know which you prefer and I'll create the appropriate PR (though as you point out, the PR for the former will be a 1 line change in build.gradle).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants