-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
|
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.
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.
@@ -78,7 +78,7 @@ public final class ActorQueue<ActorType: Actor> { | |||
|
|||
// 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
" -> "enqueue
ed and enqueueAndWait
ed"?
@@ -54,7 +54,7 @@ public final class FIFOQueue: Sendable { | |||
/// Schedules an asynchronous task for execution and immediately returns. | |||
/// 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!
@@ -36,46 +36,46 @@ final class FIFOQueueTests: XCTestCase { | |||
|
|||
// 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