Skip to content

Conversation

@twissel
Copy link

@twissel twissel commented Dec 26, 2019

@withoutboats need your opinion about my tryout with nix types

@withoutboats
Copy link
Collaborator

Probably I can't review this until after the new year, but superficially its exciting. Thanks :)

@withoutboats
Copy link
Collaborator

Ahh I see now that this was supposed to supercede #26. Reviewing fully now

Copy link
Collaborator

@withoutboats withoutboats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this all looks great! Thanks so much, I'll be excited to add it. Only changes I propose are small stylistic ones.

Sorry again for letting it sit a month!

}
}

pub unsafe fn as_socket_addr(&self) -> io::Result<SockAddr> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to make sure I have a correct understanding of this API: this function is safe to call after as the prep_accept SQE you passed this to has completed, and calling it before it has completed would be incorrect. Is that right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely right.

let mut ring = iou::IoUring::new(2)?;
let (read, _write) = net::UnixStream::pair()?;
let uname = nix::sys::utsname::uname();
let version = semver::Version::parse(uname.release());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good way to solve running tests only under certain versions of Linux, a problem also discussed in #6 and #17. After we merge this, it would be good to track developing a lightweight API for running tests differently before/after a certain version string.

Dmitriy added 2 commits January 22, 2020 21:54
# Conflicts:
#	Cargo.toml
#	src/lib.rs
#	src/sqe.rs
#	tests/poll.rs
@twissel twissel marked this pull request as ready for review January 22, 2020 19:37
@withoutboats withoutboats merged commit 17de5a5 into ringbahn:master Jan 23, 2020
@withoutboats
Copy link
Collaborator

Merged into master! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants