-
-
Notifications
You must be signed in to change notification settings - Fork 14
async -> enqueue. await -> enqueueAndWait #12
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
Conversation
2b8f252 to
3454ea6
Compare
Codecov Report
@@ Coverage Diff @@
## main #12 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 7 7
Lines 752 752
=========================================
Hits 752 752
|
omv-libs
left a comment
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.
I like this naming better. The prior version had too much baggage from the other APIs that use the same names.
bachand
left a comment
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.
Nice. I agree it's good to pick unique names that don't overlap with Swift language names.
| // MARK: Public | ||
|
|
||
| /// Sets the actor context within which each `async` and `await`ed task will execute. | ||
| /// Sets the actor context within which each `enqueue` and `enqueueAndWait` task will execute. |
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.
"enqueue and enqueueAndWait" -> "enqueueed and enqueueAndWaited"?
| /// The scheduled task will not execute until all prior tasks – including suspended tasks – have completed. | ||
| /// - Parameter task: The task to enqueue. | ||
| public func async(_ task: @escaping @Sendable () async -> Void) { | ||
| public func enqueue(_ task: @escaping @Sendable () async -> Void) { |
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.
Do we need to edit this await in this line?
| /// Asynchronous tasks sent to this queue work as they would in a `DispatchQueue` type. Attempting to `await` this queue from a task executing on this queue will result in a deadlock. |
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.
oooooh solid find!
| systemUnderTest = FIFOQueue() | ||
| } | ||
|
|
||
| // MARK: Behavior Tests |
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.
There are at least a couple places in the file like this that we should clean up.
| // You can prove this to yourself by replacing `systemUnderTest.async` above with `Task`. |
|
Pushed feedback up into #13 |
Resolves #10