Skip to content

Conversation

@florina-muntenescu
Copy link
Collaborator

No description provided.

@florina-muntenescu florina-muntenescu force-pushed the step4/database branch 2 times, most recently from 24273d4 to 1265543 Compare April 15, 2020 18:00
@florina-muntenescu florina-muntenescu force-pushed the step4/database branch 2 times, most recently from dc004a5 to adba8cd Compare April 26, 2020 15:01
}
LoadType.START -> {
val remoteKeys = getRemoteKeyForFirstItem(state)
if (remoteKeys == null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could replace this with a checkNotNull which might be more idiomatic, it throws IllegalStateException which might be more apt here though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Although this will be obsolete once yigit's change lands.

https://android-review.googlesource.com/c/platform/frameworks/support/+/1295842

Might want to just wait for this.

}
LoadType.END -> {
val remoteKeys = getRemoteKeyForLastItem(state)
if (remoteKeys?.nextKey == null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above about checkNotNull from kotlin stdlib.

try {
val apiResponse = service.searchRepos(apiQuery, page, state.config.pageSize)

return if (apiResponse.isSuccessful) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still gain anything by using the callback call adapter if we still need to check for HttpException and IOException? I feel like we should just use the suspend adapter from Retrofit, since that will likely be more relatable to users who are interested in using Coroutines. Sorry to flip back and forth on this, chris may disagree with me here though and in that case we can just keep this :D

@florina-muntenescu florina-muntenescu deleted the step4/database branch May 5, 2020 14:05
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

Successfully merging this pull request may close these issues.

2 participants