Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Some small updates so it still works fine :) #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

istvangal
Copy link

Based on sgarfinkel's RxJava2 updates (thanks for that!) I also added some minor fixes so the lib and app would still work fine as of today.

sgarfinkel and others added 3 commits September 28, 2017 23:50
- fixed RxJava2 NPE
- added location access request
- updated some dependencies
@mrmitew
Copy link
Owner

mrmitew commented Jun 29, 2018

Hey, thank you guys! I will check the pull requests and merge them to the repo :)

@@ -116,12 +124,12 @@ public void onFailure(int reason) {
* @return a {@link Single} observable that emits a null value for successful discovery, or
* throws an error
*/
public Single<Void> singleDiscoverPeers() {
public Single<Object> singleDiscoverPeers() {
Copy link

@diegotori diegotori Jul 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should convert this to Completable since that is the recommended way to represent Observable<Void> without having to ever return null.

Also, Single easily converts to Completable via toCompletable() method.

@@ -220,7 +228,7 @@ public void onFailure(int reason) {
* @return a {@link Single} observable that emits the intent which indicated that p2p peers
* changed
*/
private Single<Intent> listenForNewPeers(Single<Void> voidSingle) {
private Single<Intent> listenForNewPeers(Single<Object> voidSingle) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be affected by the above comment regarding converting this to a Completable. Perhaps it might make sense to convert this to an ObservableTransformer.

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

Successfully merging this pull request may close these issues.

4 participants