Skip to content
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 more accessors for AioCb #773

Merged
merged 1 commit into from
Oct 7, 2017
Merged

Conversation

asomers
Copy link
Member

@asomers asomers commented Sep 28, 2017

No description provided.

Copy link
Contributor

@Susurrus Susurrus left a comment

Choose a reason for hiding this comment

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

Looks pretty good aside from my two comments.

src/sys/aio.rs Outdated

/// Returns the desired length of the aio operation in bytes
///
/// If the operation has completed, this does *not* return the completed
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we embed this into the type system somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but I could probably word it better.

assert_eq!(99, sev.sigev_value.sival_ptr as i64);
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this extra newline.

@asomers
Copy link
Member Author

asomers commented Oct 7, 2017

I'm going to consider @Susurrus's statement as a conditional approval. I fixed the newline and updated the description of nbytes

bors r+ susurrus

bors bot added a commit that referenced this pull request Oct 7, 2017
773: Add more accessors for AioCb r=asomers a=asomers
@bors
Copy link
Contributor

bors bot commented Oct 7, 2017

@bors bors bot merged commit 6bb3704 into nix-rust:master Oct 7, 2017
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