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

Make sync::watch::Sender::subscribe public #3800

Merged
merged 5 commits into from
Aug 19, 2021

Conversation

theduke
Copy link
Contributor

@theduke theduke commented May 18, 2021

Motivation

Unlike with sync::broadcast::Sender, the sync::watch::Sender::subscribe method is currently not public.

This means handing out new subscriptions means keeping around a redundant Receiver, which is pretty unergonomic.

I don't really see a reason why this is the case, since the data structuredoesn't seem to have any relevant restrictions.

This was also requested in #3757.

Solution

Make watch::Sender::subscribe public.

Also adds a doc test.

Make the internal watch::Sender::subscribe method public.
Also adds documentation.
@theduke
Copy link
Contributor Author

theduke commented May 18, 2021

Note: the subscribe method was gated on the signal feature.

I don't see why this was the case, maybe because it was only required in other code behind the feature?

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels May 19, 2021
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment on lines 412 to 413
/// Creates a new [`Receiver`] handle that will receive values sent **after**
/// this call to `subscribe`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Creates a new [`Receiver`] handle that will receive values sent **after**
/// this call to `subscribe`.
/// Creates a new [`Receiver`] handle that will receive values sent after
/// this call to `subscribe`.

Comment on lines 412 to 415
/// Creates a new [`Receiver`] handle that will receive values sent **after**
/// this call to `subscribe`.
///
/// # Examples
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a note about how this method behaves if the channel is closed when you call it? And preferably also add a test for that behavior?

@Darksonn
Copy link
Contributor

What's the status on this PR?

@theduke
Copy link
Contributor Author

theduke commented Jun 16, 2021

I'll get to to it in the next few days - today or tomorrow hopefully.

@Darksonn Darksonn changed the title Make sync::watch::Sender::subscribe public Make sync::watch::Sender::subscribe public Aug 17, 2021
@Darksonn Darksonn merged commit 8aa2bfe into tokio-rs:master Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-sync Module: tokio/sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants