-
Notifications
You must be signed in to change notification settings - Fork 718
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
Split Event::is_hup into is_hup and is_read_hup and refer to OS selector docs #1037
Conversation
And refer to the selector documentation.
ef8f4a8
to
c42e2db
Compare
Rebased on master. |
/// | [OS selector] | Flag(s) checked | | ||
/// |---------------|-----------------| | ||
/// | [epoll] | `EPOLLRDHUP` | | ||
/// | [kqueue] | `EV_EOF` | |
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 is: EVFILT_READ
with EV_EOF
.
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.
You're right but doesn't the read part of the method name imply that? I'm fine with adding it though.
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, some doc thoughts inline.
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.
Quick thing I missed.
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.
Excellent, thanks!
This doesn't mention Windows, I think the Windows rewrite should be merged first.
Updates #941.
Closes #1040.