-
Notifications
You must be signed in to change notification settings - Fork 589
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
Migrate to RxJava 3 #722
Migrate to RxJava 3 #722
Conversation
Thanks for helping with the work. Unfortunately I cannot accept this PR until you will sign the CLA. If you would have time to fix imports, docs and change package name to |
@dariuszseweryn I'm struggling at the moment to build the library to my local maven repository. Generally edit: I've manually edited the pom to include the dependency information but this is less than optimal |
@dariuszseweryn ping? |
Any news here ? because RxJava2 will soon be not supported anymore : https://github.com/ReactiveX/RxJava#version-2x |
I've noticed when using the lib built on jitpack.io (and manually adding back the dependencies, see my comments above) this:
Stacktrace:
|
The previous comment seems to be caused by https://issuetracker.google.com/issues/183632445 , workaround there is to put |
FYI I'll probably only be able to work on this until the end of May, after that somebody else will have to complete the PR. |
@z3ntu @dariuszseweryn Any update on this PR? 🙏🏼 |
@z3ntu I wanted to continue your PR but, I get this: ERROR: Permission to z3ntu/RxAndroidBle.git denied to Drjacky.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists. seems you need to check something like
|
@Drjacky You're not maintainer which is why you don't have permission to push. The checkbox was on. I've given you direct access now though. Feel free to rebase and fix up the rest etc. I've kept the last couple of commits un-squashed on purpose to allow to reproduce them in case upstream changes (which it did again as you can see). When it's getting close to be merged (maybe this will happen at some point..?) then just squash them together but please keep me as author. Good luck :) |
@dariuszseweryn Please check Readme file to see if you're happy with Also, in
Let me know if you want me to change them to something. |
@dariuszseweryn @z3ntu Anything else we need to do to have this PR to be merged? |
Co-authored-by: Luca Weiss <[email protected]>
According to the below link some test assertions are now not available. Made substitute extensions. ReactiveX/RxJava#6526
RxJava 3 removes quite a lot of TestObserver’s functions. This commit re-adds them as extensions for kotlin sample tests purposes.
git grep io.reactivex | grep -v rxjava3
find . -name rxandroidble2 | rev | cut -c2- | rev | xargs -I % mv "%2" "%3"
git grep -l rxandroidble2 | xargs sed -i 's|rxandroidble2|rxandroidble3|g'
I've resolved the conflicts. |
@dariuszseweryn Is there any plan to update this amazing library with RxJava 3? I'm really looking forward to it 🔥 |
I have still this in plans. I am currently working on merging #770 which should unlock me a bit but I must keep the ability to release the library |
Waiting for this PR merging. Thanks |
I decided to fork this repository due to lack of activity. I have merged this PR (and others) and released as |
After merging #793 RxJava 3 based dependency is now on Maven Central under |
Based on
test/rxjava3
branch with updated imports & dependency updates for rxjava3.As far as I can tell, all the tests are succeeding.
What's missing:
Some test files still contain "old imports" as function names, e.g.NotificationAndIndicationManagerTest.groovy
Some doc strings still have the old imports, e.g.BleAlreadyConnectedException.java
Fixes #690