-
-
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
io: clarify clear_ready
and clear_ready_matching
documentation
#6304
Conversation
tokio/src/io/async_fd.rs
Outdated
/// Note that that if a readiness notification occurs following the last operation | ||
/// but prior to invoking `clear_ready`, it will not be cleared. |
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 think it's really important to be very clear in the wording here, because the point we are making is tricky to understand.
Here's one suggested wording. This one tries to explain it twice with "in other words". I still don't love it though.
/// Note that that if a readiness notification occurs following the last operation | |
/// but prior to invoking `clear_ready`, it will not be cleared. | |
/// This method only clears readiness events that happened before the creation of this guard. In other words, if the IO resource becomes ready between the creation of the guard and this call to `clear_ready`, then the readiness is not actually cleared. |
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 actually like that the timing is tied to the creation of the guard. I believe it makes it clear.
tokio/src/io/async_fd.rs
Outdated
/// This implies that if a readiness notification occurs following the last operation | ||
/// but prior to invoking `clear_ready`, it will not be cleared. | ||
/// |
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 part isn't on all of them? But I think the other change is probably sufficient.
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.
Ah yes it was a leftover comment. Thanks.
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.
Resolves #6302.