-
Notifications
You must be signed in to change notification settings - Fork 797
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
Cancellable: remove UsingToken usages in tests #18276
Conversation
✅ No release notes required |
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.
Accidental comment from the release notes editor, I don't seem to be able to delete it.
So, just removing the member from the public API didn't work, as it was used in |
@auduchinok I'm not sure about removing inline there, would it affect stack traces and debugging? Somewhat related: It would be great to integrate the functionality of |
54bf644
to
f41696e
Compare
The token is now always set inside the cancellable computation
f41696e
to
91ef654
Compare
So, I did some benchmark runs using FCSSourceFiles benchmark and frankly I see no difference between inlined and not inlined /// Run a cancellable computation using the given cancellation token
let inline run (ct: CancellationToken) (Cancellable oper) =
if ct.IsCancellationRequested then
ValueOrCancelled.Cancelled(OperationCanceledException ct)
else
oper ct it seems the idea was to just expand the cancel check inline. |
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
Removes
Cancellable.UsingToken
from the public surface, since the computation now always sets the token.