-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 AsyncFd::with_interest #3167
Add AsyncFd::with_interest #3167
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.
LGTM, I opened a minor naming point.
tokio/src/io/async_fd.rs
Outdated
pub(crate) fn new_with_handle(inner: T, handle: Handle) -> io::Result<Self> { | ||
/// Creates new instance as `new` with additional ability to customize interest, | ||
/// allowing to specify whether file descriptor will be polled for read, write or both. | ||
pub fn new_with_interest(inner: T, interest: Interest) -> io::Result<Self> |
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.
Thanks, I'm wondering if this should just be with_interest
(modeling after Vec::with_capacity
).
The other new_with
fns are internal and haven't received as much attention naming wise.
Thoughts @tokio-rs/maintainers ?
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 have a slight preference for with_interest
, but i could be convinced otherwise.
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.
Changed it to with_interest
, it is shorter too so I would prefer it
} | ||
|
||
pub(crate) fn new_with_handle(inner: T, handle: Handle) -> io::Result<Self> { | ||
pub(crate) fn new_with_handle_and_interest( |
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.
We may also want to rename this to with_handle_and_interest
for consistency, but definitely not a blocker. I will merge this shortly if nobody has any other comments.
Expose
Interest
customization forAsyncFd
Fixes #3072