-
Notifications
You must be signed in to change notification settings - Fork 54
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
Removed the Task trait #1878
Removed the Task trait #1878
Conversation
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.
The changes looked fine to me, but I'll let @didier-wenzek approve as the runtime is his forte.
/// | ||
/// // The actor is then spawn in the background with its message box. | ||
/// tokio::spawn(actor.run()); | ||
/// tokio::spawn(async move { actor.run().await } ); |
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've always wondered what the difference between these 2 variations of spawning an sync function, as I've always used the async {}
block variant. So, in the previous version where the actor.run()
was called directly, the move
was implicit because that function was taking ownership of self
?
Robot Results
Passed 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.
Approved
actor: Box<dyn Actor>, | ||
runtime_request_sender: DynSender<RuntimeRequest>, | ||
) -> Self { | ||
let name = format!("actor '{}'", actor.name()); |
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.
Why not simply take the actor's name?
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.
This was already there I'll change it.
952c239
to
3853203
Compare
Proposed changes
Types of changes
Paste Link to the issue
Checklist
cargo fmt
as mentioned in CODING_GUIDELINEScargo clippy
as mentioned in CODING_GUIDELINESFurther comments