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

feat(mpsc): add len, capacity, and remaining methods to mpsc #72

Merged
merged 14 commits into from
Apr 6, 2024

Conversation

jcrevier
Copy link
Contributor

@jcrevier jcrevier commented Oct 5, 2022

Fixes #71

@jcrevier jcrevier changed the title feat(mpsc): add len, capacity, and remaining methods to mpsc (#71) feat(mpsc): add len, capacity, and remaining methods to mpsc Oct 5, 2022
@jcrevier jcrevier force-pushed the feature/channel-len branch from 34b0e9e to 6b7f22f Compare October 5, 2022 00:31
Copy link
Owner

@hawkw hawkw 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 looks good to me, thanks! i commented on a few minor suggestions inline.

also, I notice that this branch only adds these methods to the mpsc::Sender and mpsc::Receiver types. can we also add similar methods to the other channel flavors:

i realize this is a bunch of additional code to add, but for the most part, it should be possible to copy and paste the code from mpsc/async_impl.rs to the other files, with minor changes to the doctests to import and construct the appropriate types.

thank you!

///
/// [`remaining`]: Self::remaining
#[inline]
pub fn capacity(&self ) -> usize {
Copy link
Owner

Choose a reason for hiding this comment

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

tiny style nit:

Suggested change
pub fn capacity(&self ) -> usize {
pub fn capacity(&self) -> usize {

src/mpsc/async_impl.rs Show resolved Hide resolved
///
/// [`remaining`]: Self::remaining
#[inline]
pub fn capacity(&self ) -> usize {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
pub fn capacity(&self ) -> usize {
pub fn capacity(&self) -> usize {

///
/// [`remaining`]: Self::remaining
#[inline]
#[allow(clippy::len_without_is_empty)]
Copy link
Owner

Choose a reason for hiding this comment

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

i think it would be fine to also add an is_empty method :)

Copy link
Owner

Choose a reason for hiding this comment

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

and also

Suggested change
#[allow(clippy::len_without_is_empty)]
#[allow(clippy::len_without_is_empty)]
#[must_use]

///
/// [`remaining`]: Self::remaining
#[inline]
#[allow(clippy::len_without_is_empty)]
Copy link
Owner

Choose a reason for hiding this comment

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

as above, i think we can just add an is_empty method to this as well.

Copy link
Owner

Choose a reason for hiding this comment

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

and

Suggested change
#[allow(clippy::len_without_is_empty)]
#[allow(clippy::len_without_is_empty)]
#[must_use]

src/mpsc/async_impl.rs Show resolved Hide resolved
src/mpsc/async_impl.rs Show resolved Hide resolved
src/mpsc/async_impl.rs Show resolved Hide resolved
src/mpsc/async_impl.rs Show resolved Hide resolved
@jcrevier
Copy link
Contributor Author

jcrevier commented Oct 5, 2022

Ah, didn't notice that there was a StaticReceiver and StaticSender, definitely will want the methods on those. No qualms with any of the other suggestions. I'll update the PR shortly.

@hawkw
Copy link
Owner

hawkw commented Oct 5, 2022

Great, thank you! By the way, PR #73 fixed the build failure due to Clippy lints, so you may want to rebase onto the latest main so we can get a clean build. :)

Thanks again!

@jcrevier jcrevier force-pushed the feature/channel-len branch from 659147a to 6b5e0c3 Compare October 6, 2022 19:18
@jcrevier jcrevier force-pushed the feature/channel-len branch from 6b5e0c3 to cfd9439 Compare October 6, 2022 19:21
Copy link
Owner

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

this looks good to me, thank you!

one last docs nit: it looks like the documentation for the StaticReceiver/StaticSender methods links to the non-static Sender/Receiver types. i'm going to just go ahead and fix that, and merge this branch.

thanks again for the PR!

src/mpsc/blocking.rs Outdated Show resolved Hide resolved
src/mpsc/blocking.rs Outdated Show resolved Hide resolved
src/mpsc/blocking.rs Outdated Show resolved Hide resolved
src/mpsc/blocking.rs Outdated Show resolved Hide resolved
src/mpsc/blocking.rs Outdated Show resolved Hide resolved
src/mpsc/async_impl.rs Outdated Show resolved Hide resolved
src/mpsc/async_impl.rs Outdated Show resolved Hide resolved
src/mpsc/async_impl.rs Outdated Show resolved Hide resolved
src/mpsc/async_impl.rs Outdated Show resolved Hide resolved
src/mpsc/async_impl.rs Outdated Show resolved Hide resolved
@hawkw
Copy link
Owner

hawkw commented Oct 6, 2022

ugh, i seem to have messed something up, hang on...

src/mpsc/async_impl.rs Outdated Show resolved Hide resolved
src/mpsc/async_impl.rs Outdated Show resolved Hide resolved
@hawkw hawkw enabled auto-merge (squash) April 6, 2024 17:07
hawkw added 2 commits April 6, 2024 10:24
these tests drop the rx, closing the channel. thus, the test fails.
@hawkw hawkw disabled auto-merge April 6, 2024 17:47
@hawkw hawkw enabled auto-merge (squash) April 6, 2024 17:52
@hawkw hawkw merged commit 00213c1 into hawkw:main Apr 6, 2024
25 checks passed
@jcrevier jcrevier deleted the feature/channel-len branch April 6, 2024 18:43
hawkw added a commit that referenced this pull request Apr 6, 2024
## v0.1.5 (2024-04-06)

#### Features

* **mpsc:**  add `len`, `capacity`, and `remaining` methods to mpsc (#72) ([00213c1](00213c1), closes [#71](#71))

#### Bug Fixes

*   unused import with `alloc` enabled ([ac1eafc](ac1eafc))
*   skip slots with active reading `Ref`s in `push_ref` (#81) ([a72a286](a72a286), closes [#83](#83), [#80](#80))
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.

Adding remaining() and capacity() methods to Sender and Receiver
2 participants