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

Support for rx.Single #573

Merged
merged 1 commit into from Jan 12, 2016
Merged

Support for rx.Single #573

merged 1 commit into from Jan 12, 2016

Conversation

geralt-encore
Copy link
Collaborator

@geralt-encore geralt-encore changed the title Support for Single Support for rx.Single Dec 20, 2015
contentResolver.insert(TestItem.CONTENT_URI, testItemToInsert.toContentValues());

Cursor firstDbState = contentResolver.query(TestItem.CONTENT_URI, null, null, null, null);
Assertions.assertThat(firstDbState).hasCount(1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

With static import it will be more concisely =) Here and below in tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this file, there are two kinds of assertThat: from org.assertj.android.api and org.assertj.core.api. So static import can't be used for both of them.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that api design problems, why it's not androidAssertThat()

Copy link
Collaborator

Choose a reason for hiding this comment

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

🐼

@nikitin-da
Copy link
Collaborator

Wow such huge PR! 👍

@@ -37,4 +38,14 @@
@NonNull
@CheckResult
Observable<Result> createObservable();

/**
* Creates {@link rx.Single} that emits result of Operation.
Copy link
Member

Choose a reason for hiding this comment

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

Can you please change to that emits result of Operation lazily when somebody subscribes to it

@@ -81,6 +83,15 @@ public DeleteResult executeAsBlocking() {
.subscribeOn(Schedulers.io());
}

@NonNull
@Override
public Single<DeleteResult> asRxSingle() {
Copy link
Member

Choose a reason for hiding this comment

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

@CheckResult

@nikitin-da
Copy link
Collaborator

LGTM

@geralt-encore
Copy link
Collaborator Author

Squashed and updated

@artem-zinnatullin
Copy link
Member

Phew…

artem-zinnatullin added a commit that referenced this pull request Jan 12, 2016
@artem-zinnatullin artem-zinnatullin merged commit 9e6f9ed into pushtorefresh:master Jan 12, 2016
@artem-zinnatullin
Copy link
Member

Dat thing was HUGE!

Thanks a lot, @geralt-encore! Please make smaller PRs :D

@geralt-encore
Copy link
Collaborator Author

@artem-zinnatullin Sure, but I kinda have no choice. I wanted to cover it with tests as much as possible and didn't want to do it halfway, because, probably I wouldn't come back to it otherwise.

@artem-zinnatullin
Copy link
Member

Yeah, I understand your points, anyway, great work!

@geralt-encore geralt-encore deleted the rx-single branch March 19, 2016 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants