-
Notifications
You must be signed in to change notification settings - Fork 576
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
Add Task Cancellation #89
Add Task Cancellation #89
Conversation
@josephearl This looks amazing! Keep it up and i am looking really forward to see more of these awesome things! |
Is there any movements on this PR? I'd love to have cancellation support 🌞 |
I'll be updating it shortly |
@josephearl thanks! |
Updated. Also added the ability to register a delegate to be notified when a token is cancelled. |
@nlutsenko-fb can you re-run the build please?
|
Doing this right now... Thanks! |
@josephearl this looks even better! Also - it looks like there is a test failing :( |
|
#import <Foundation/Foundation.h> | ||
|
||
/*! | ||
Represents the registration of a delegate with a cancellation token. |
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.
registration of a delegate
registration of a cancellation observer
Hey @josephearl looks it won't merge automatically (needs rebasing). |
Will do |
Rebased and moved logic from CancellationTokenSource to CancellationToken, to match TaskCompletionSource and Task. |
@josephearl This looks great! Few comments, but I am very tempted to merge it in... |
Will fix up this evening |
Updated the docs and removed the token getter |
This looks great! Merging in! |
Great job! thanks! I suggest the "Task Cancellation" described in the Readme.md file should be updated with this implementation. |
great addition! :) @nlutsenko @josephearl can I suggestion to make the cancellation token as the third parameter instead of 2nd? It can make the chaining method looks better. not sure how Android or C# doing with it. change FROM
TO
|
We can do an extra one, since these are already here and we probably don't want to break existing users :( |
Yes was just about to say we probably don't want to break the API. Sounds like a good addition. |
@josephearl updated the pull request. |
Initial CancellationToken & CancellationTokenSource implementation to match the C# API (system.threading.cancellationtoken and system.threading.cancellationtokensource) and the cancellation API on Android (BoltsFramework/Bolts-Android#53 and BoltsFramework/Bolts-Android#56).