Use libc_enum! where possible #792
Conversation
|
My editor used rustfmt, so this change is bigger than necessary. Should I undo the stuff rustfmt changed? |
|
There's no problem with those changes being the PR, though I don't agree with all of the changes. In general when contributing PRs, however, you should separate changes into multiple commits to make it easier to review. In this case, I'd suggest one commit for the functional code changes and one commit for all the remaining style changes. |
|
OK, I removed the formatting changes for now. |
src/sys/aio.rs
Outdated
| /// do it like `fsync` | ||
| O_SYNC, | ||
| /// on supported operating systems only, do it like `fdatasync` | ||
| #[cfg(any(target_os = "openbsd", target_os = "bitrig", |
There was a problem hiding this comment.
Since you're touching these lines I'd ask that you also alphabetize the target OSes and when they're longer than a line and need to be wrapped that you wrap them such that there's 1 OS per line. This applies across this whole PR.
|
You'll need to rebase now that #790 was merged. |
Some enums which use different names for values than libc still set the discriminators manually. closes nix-rust#254
src/sys/aio.rs
Outdated
| #[cfg(any(target_os = "openbsd", target_os = "bitrig", | ||
| target_os = "netbsd", target_os = "macos", target_os = "ios", | ||
| target_os = "linux"))] | ||
| #[cfg(any( |
There was a problem hiding this comment.
Can you put the first target OS on the same line as #[cfg(any(? This is consistent with our styling elsewhere.
|
OK, I will try to take care of this in ~10 hours. |
|
Please squash your last two commits into one as they're both part of the same change. I believe the test failures on Darwin targets were changes in Cross that have since been resolved, but I'm testing now. So with those things fixed, we can merge this. |
|
@wginolas Just needs those last 2 commits squashed and then this can be merged! |
|
bors r+ |
Some enums which use different names for values than libc still set the
discriminators manually.
closes #254