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

Fix Issue #1608 - retry() failing in FirestorePagingAdapter #1609

Merged
merged 3 commits into from Mar 28, 2019

Conversation

@PatilShreyas
Copy link
Contributor

PatilShreyas commented Mar 27, 2019

I have fixed retry() failing and added a new method refresh() to refresh the list from the beginning as issue #1608

@PatilShreyas PatilShreyas requested a review from samtstern as a code owner Mar 27, 2019
Copy link
Member

samtstern left a comment

Approve with one small change.

private final Observer<FirestoreDataSource> mDataSourceObserver = new Observer<FirestoreDataSource>() {
@Override
public void onChanged(@Nullable FirestoreDataSource source) {

This comment has been minimized.

Copy link
@samtstern

samtstern Mar 27, 2019

Member

Can we add a comment inside here explaining why we need an empty observer so that future people who read this code know?

This comment has been minimized.

Copy link
@PatilShreyas

PatilShreyas Mar 28, 2019

Author Contributor

@samtstern yes.
What comment should be added ?
Please suggest me.

This comment has been minimized.

Copy link
@PatilShreyas

PatilShreyas Mar 28, 2019

Author Contributor

//Observer to hold the value for FirestoreDataSource ??

@PatilShreyas

This comment has been minimized.

Copy link
Contributor Author

PatilShreyas commented Mar 28, 2019

Suggest me comment to write.

@samtstern

This comment has been minimized.

Copy link
Member

samtstern commented Mar 28, 2019

@PatilShreyas

This comment has been minimized.

Copy link
Contributor Author

PatilShreyas commented Mar 28, 2019

Fine!
I'm adding this.

@PatilShreyas

This comment has been minimized.

Copy link
Contributor Author

PatilShreyas commented Mar 28, 2019

@samtstern We should add this in FirebaseRecyclerPagingLibrary too.
Is it OK to make changes in FirebaseRecyclerPagingAdapter in this PR?

@samtstern

This comment has been minimized.

Copy link
Member

samtstern commented Mar 28, 2019

@PatilShreyas yep that's fine with me. I'll wait to merge until those changes are made also

Updated a comment for Observer.
@samtstern samtstern merged commit 154904e into firebase:version-4.4.0-dev Mar 28, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
cla/google All necessary CLAs are signed
@samtstern samtstern added this to the 4.4.0 milestone Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.