Use flume instead of crossbeam-channel #9
Conversation
lu-zero
force-pushed
the
flume
branch
2 times, most recently
from
f22bb4a to
58363f9
Compare
There was a problem hiding this comment.
Thanks for the PR, this looks awesome! Some of the API return types would become flume::Receiver but the change will be worth it. I only have a couple of minor comments inline.
(A related thought is that, we don't have tests or example to cover the async use case yet. That's okay for us now because otherwise we will have to drag in dev-dependencies for async runtime, making this crate much larger. The overhead feels too big for a relatively small benefit.)
| @@ -18,7 +18,7 @@ jobs: | |||
| uses: actions-rs/toolchain@v1 | |||
| with: | |||
| profile: minimal | |||
| toolchain: 1.46.0 | |||
| toolchain: 1.49.0 | |||
There was a problem hiding this comment.
I guess flume requires 1.49 and up? If so, could you please also update the version number in README.md ?
Cargo.toml
Outdated
| log = "0.4.14" # logging | ||
| nix = "0.23.0" # socket APIs | ||
|
|
||
| [dependencies.flume] |
There was a problem hiding this comment.
Just to have a consistent style, could you please define this in the [dependencies] section above?
Makes even easier to use the library from an async context. Bump the MSRV 1.49.0 since a flume dependency relies on a dependency that uses `hint::spin_loop`.
This way it is not needed to know which is the channel implementation.
Makes even easier to use the library from an async context.