Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upSplit channel.rs into mpsc and ipc files #597
Conversation
| } | ||
| } | ||
| #[cfg(feature = "ipc")] | ||
| include!("channel_ipc.rs"); |
This comment has been minimized.
This comment has been minimized.
emilio
Nov 25, 2016
Member
I'm definitely fine with this, though maybe this could be a bit more idiomatic doing something like:
#[cfg(feature = "ipc")]
mod channel_ipc;
#[cfg(feature = "ipc")]
pub use channel_ipc::*;(And the same with the mpsc).
It duplicates the cfg, but avoids macros (which may be a bit nicer IMO). What do you think? Happy to concede if people have different opinions on it :)
Also, if you could add a line to .travis.yml to test both configs as part of this PR, that would be awesome.
Thanks!
This comment has been minimized.
This comment has been minimized.
dignifiedquire
Nov 26, 2016
Author
Contributor
Added tests to travis :)
I'm happy to change the imports. I saw the include! already being used in lib.rs so I thought it would be fine (and it was the first way I figured out to make the compiler happy :)
I don't have any strong opinions either way as I am just starting to figure out my way around Rust .
This comment has been minimized.
This comment has been minimized.
dignifiedquire
Nov 26, 2016
Author
Contributor
I am getting error: cannot declare a new module at this location when trying to use the declarations suggested above in channel.rs. Not sure how to make this work, any tips?
This comment has been minimized.
This comment has been minimized.
dignifiedquire
force-pushed the
dignifiedquire:refactor/split-channel
branch
from
7ee7832
to
c7de564
| - (cd webrender && cargo test --verbose) | ||
| - (cd webrender && cargo test --verbose --features "ipc") |
This comment has been minimized.
This comment has been minimized.
emilio
Nov 26, 2016
Member
webrender and sample don't have the ipc feature, this is only a webrender_traits thing, so this line and the one below you added should go away.
This comment has been minimized.
This comment has been minimized.
| @@ -12,6 +12,9 @@ addons: | |||
| - libgles2-mesa-dev | |||
| script: | |||
| - (cd webrender_traits && cargo test --verbose) | |||
| - (cd webrender_traits && cargo test --verbose --features "ipc") | |||
This comment has been minimized.
This comment has been minimized.
emilio
Nov 26, 2016
Member
Please put this line the first one in the script to avoid unnecessary recompiles.
This comment has been minimized.
This comment has been minimized.
dignifiedquire
force-pushed the
dignifiedquire:refactor/split-channel
branch
from
c7de564
to
7911812
|
Looks good, thanks! |
| } | ||
| } | ||
| #[cfg(feature = "ipc")] | ||
| include!("channel_ipc.rs"); |
This comment has been minimized.
This comment has been minimized.
|
@bors-servo r+ |
|
|
|
|
Split channel.rs into mpsc and ipc files Fixes #573 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/597) <!-- Reviewable:end -->
dignifiedquire commentedNov 25, 2016
•
edited by larsbergstrom
Fixes #573
This change is