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

Document how BoxFutures / BoxStreams are often made #2887

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Oct 3, 2024

Rationale

Unless you are already familiar with the Rust futures ecosystem and FuturesExt::boxed it is not always obvious how to create a BoxFuture

Specifically, we have APIs for reading parquet files, which involve BoxFutures that when a user clicks on the return type,

Screenshot 2024-10-03 at 9 22 37 AM

The documentation doesn't lead them to FutureExt::boxed:

Screenshot 2024-10-03 at 9 21 43 AM

Proposal

While a better improvement for our users is a full featured example (added in apache/arrow-rs#6505), I think this to propose improvements in this crate too to help the broader community

If the maintainers like this pattern, I am happy to add it for other similar types in Futures and Streams but I won't bother if you are not interested in this type of change

@@ -9,6 +9,11 @@ pub use core::future::Future;

/// An owned dynamically typed [`Future`] for use in cases where you can't
/// statically type your result or need to add some indirection.
///
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This follows the wording and pattern in std, for example https://doc.rust-lang.org/std/iter/struct.Cloned.html

This struct is created by the cloned method on Iterator. See its documentation for more.

Here is how it looks rendered:
Screenshot 2024-10-03 at 9 34 52 AM

/// This type is often created by the [`boxed`] method on [`FutureExt`]. See its documentation for more.
///
/// [`boxed`]: https://docs.rs/futures/latest/futures/future/trait.FutureExt.html#method.boxed
/// [`FutureExt`]: https://docs.rs/futures/latest/futures/future/trait.FutureExt.html
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this links back to docs.rs because futures-core does not depened on futures-util, where FutureExt is defined

@alamb
Copy link
Contributor Author

alamb commented Oct 3, 2024

FYI @taiki-e

@taiki-e
Copy link
Member

taiki-e commented Oct 3, 2024

Thanks! This seems reasonable to me.

@taiki-e taiki-e added docs 0.3-backport: pending The maintainer accepted to backport this to the 0.3 branch, but backport has not been done yet. labels Oct 3, 2024
@taiki-e
Copy link
Member

taiki-e commented Oct 3, 2024

Could you add similar docs to other boxed types?

@alamb
Copy link
Contributor Author

alamb commented Oct 3, 2024

Could you add similar docs to other boxed types?

Yes, I will do so

@alamb alamb changed the title Document how BoxFutures are often made Document how BoxFutures / BoxStreams are often made Oct 3, 2024
@alamb
Copy link
Contributor Author

alamb commented Oct 3, 2024

Could you add similar docs to other boxed types?

Yes, I will do so

Done

@taiki-e taiki-e merged commit bbfc1ed into rust-lang:master Oct 4, 2024
24 checks passed
@alamb alamb deleted the alamb/box_future_docs branch October 4, 2024 10:58
@alamb
Copy link
Contributor Author

alamb commented Oct 4, 2024

Thank you again @taiki-e

@taiki-e taiki-e mentioned this pull request Oct 5, 2024
@taiki-e taiki-e added 0.3-backport: completed and removed 0.3-backport: pending The maintainer accepted to backport this to the 0.3 branch, but backport has not been done yet. labels Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants