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
Piped-promises for Android #65
base: master
Are you sure you want to change the base?
Conversation
hiya! could i trouble you to elaborate on the usage? also, I noticed many of the return types had changed from Promise to AndroidPromise. That's a pretty significant change that could break existing users. |
You use it pretty much the same way as new AndroidDeferredManager()
.when(new Callable<Foo>() {
@Override
public Foo call() throws Exception {
...
return ...;
}
})
...
.then(new AndroidDonePipe<Foo, Bar, Throwable, Void>() {
@Override
public AndroidPromise<Bar, Throwable, Void> pipeDone(Foo foo) {
Bar bar = someAction(foo);
...
return new AndroidDeferredObject<Bar, Throwable, Void>()
.resolve(bar);
}
@Override
public AndroidExecutionScope getExecutionScope() {
return AndroidExecutionScope.BACKGROUND;
}
})
... Another solution would have been to just extend the As for the changed return types, when you're using promises on Android I think you should always use the Android-specific promises with execution scope; using the regular promises can result in 'weird' errors because they're always executed on the UI-thread. I tried to enforce this by only allowing Android-promises in the So I guess the changed return types are not necessary for this PR, but you might consider keeping them anyway, because it helps users realize that Android-promises are the preferred choice when using |
@@ -21,7 +21,7 @@ | |||
<version>1.2.5-SNAPSHOT</version> | |||
<relativePath>../pom.xml</relativePath> | |||
</parent> | |||
<artifactId>jdeferred-android-aar</artifactId> | |||
<artifactId>jdeferred-jdeferred-android-aar</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hiya, could i trouble you to rename this back?
ping |
I added piped promises for Android as best as I could without modifying the non-Android code-base. This restriction unfortunately prohibited me from writing type-safe code (i.e. enforce use of Android-only promises when AndroidDeferredManager is used), so instead I had to fallback to enforcing the use of Android-promises by throwing exceptions whenever a non-Android promise is used in an Android-context.
Also I could not get the test-suite running in IntelliJ IDEA, so I am not sure if the single test I added even works, you will have to try that for yourself. I do use the code from this PR actively on a daily base though, which leads me to believe it works pretty well.
This change is