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

v1.0 release #49

Open
notgull opened this issue Oct 29, 2024 · 4 comments
Open

v1.0 release #49

notgull opened this issue Oct 29, 2024 · 4 comments

Comments

@notgull
Copy link

notgull commented Oct 29, 2024

I want to add a oneshot channel to smol's public API, mainly for discoverability. Rather than re-inventing the wheel and coding one, I figure why not just re-export this crate?

However smol is a stable crate, so this crate would need to have a stable (v1.0) release if that were to happen.

Are there any issues that would prevent a stable release? I'm willing to help if you'll have me.

cc @madsmtm, @smol-rs/admins

@madsmtm
Copy link

madsmtm commented Oct 29, 2024

Looking at the public API, a few thoughts come to mind:

  • Maybe make the error types opaque? Though there is a nicety in that they currently match those in std::sync::mpsc.
  • Would be nice to have #![cfg_attr(docsrs, feature(doc_auto_cfg))] to make it easier to see which things are cfg-gated behind the std feature.
  • Maybe remove the "async" feature altogether, it seems weird to have it optional when Future is available in core? If not possible because of the ReceiverWaker stuff, then maybe it makes sense to expose different functions for each of the different use-cases (would also avoid the panic if calling both poll and recv).
  • [Ref]UnwindSafe impls would be nice.

@notgull
Copy link
Author

notgull commented Oct 29, 2024

Pinging @zeenix since this has been discussed re: adding async-broadcast to smol

@faern
Copy link
Owner

faern commented Oct 29, 2024

Thanks for opening this issue. oneshot has been around for a while now. Yeah, maybe it's time to stabilize it soon?

* Maybe make the error types opaque? Though there is a nicety in that they currently match those in `std::sync::mpsc`.

Which error specifically? I think the errors currently has a pretty clean and nice API surface.

* Would be nice to have `#![cfg_attr(docsrs, feature(doc_auto_cfg))]` to make it easier to see which things are `cfg`-gated behind the `std` feature.

Indeed! Making the features more discoverable would be great. I would be all for merging something like this.

* Maybe remove the `"async"` feature altogether, it seems weird to have it optional when `Future` is available in `core`? If not possible because of the `ReceiverWaker` stuff, then maybe it makes sense to expose different functions for each of the different use-cases (would also avoid the panic if calling both `poll` and `recv`).

async being feature gated is not about dependencies, but rather compiling the code as optimally as possible. Some types and methods can get rid of some code if one or the other feature is not enabled. My main motivation for writing oneshot was to have a channel impl that could easily pass messages between async and blocking operations of a program. But it's still valid to use it within only one type, and then having it compile to whatever is optimal for only that is neat. But I have to admit that I have not benched this to validate if this is a valid reason or not. I agree that both the code and the usage can be a bit simpler if we just have it on at all times.

* `[Ref]UnwindSafe` impls would be nice.

I have to look into this. I have no opinion at this point. Do you have a use case in mind that you can describe?

@zeenix
Copy link

zeenix commented Oct 29, 2024

Pinging @zeenix since this has been discussed re: adding async-broadcast to smol

I'm not sure about the relevance here. We can add oneshot to smol, independently of async-broadcast addition.

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

No branches or pull requests

4 participants