Skip to content

Remove top-level re-exports #1855

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

Closed
wants to merge 3 commits into from

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Sep 1, 2019

Closes #1854

@Nemo157
Copy link
Member

Nemo157 commented Sep 1, 2019

What’s the feeling on removing all the doc(hidden) top-level reexports? It seems very odd to me to have undocumented exports for users to just accidentally stumble across.

@taiki-e
Copy link
Member Author

taiki-e commented Sep 1, 2019

What’s the feeling on removing all the doc(hidden) top-level reexports?

Personally I don't think I should depend on APIs that are not displayed in the documentation, so I would like to remove them, but unfortunately, some of the current documentation depends on these reexports.

It seems very odd to me to have undocumented exports for users to just accidentally stumble across.

I strongly agree.

@taiki-e taiki-e changed the title Remove top-level re-export of Poll Reduce top-level re-exports Sep 1, 2019
@taiki-e
Copy link
Member Author

taiki-e commented Sep 1, 2019

So, I added a commit to reduce some top-level re-exports and changed the title of PR.

@taiki-e

This comment has been minimized.

@taiki-e taiki-e changed the title Reduce top-level re-exports WIP: Reduce top-level re-exports Sep 1, 2019
@taiki-e

This comment has been minimized.

@taiki-e taiki-e changed the title WIP: Reduce top-level re-exports Reduce top-level re-exports Sep 1, 2019
@taiki-e
Copy link
Member Author

taiki-e commented Sep 1, 2019

Seems intra-doc has been improved. All of top-level reexports can be removed.

@taiki-e taiki-e changed the title Reduce top-level re-exports Remove top-level re-exports Sep 1, 2019
@cramertj
Copy link
Member

cramertj commented Sep 3, 2019

I personally use these pretty often and find having to import from two different modules just to get TryFutureExt and TryStreamExt pretty annoying. If no one else is bothered by this, I don't feel strongly enough to block, but I do think it's a decent-sized ergonomic hurdle.

@taiki-e
Copy link
Member Author

taiki-e commented Sep 3, 2019

@cramertj Okay, I separated the part not related to ext trait into #1858.

I personally use these pretty often and find having to import from two different modules just to get TryFutureExt and TryStreamExt pretty annoying. If no one else is bothered by this, I don't feel strongly enough to block, but I do think it's a decent-sized ergonomic hurdle.

Is this be resolved by merging try_{future, stream} modules into {future, stream} modules?

@cramertj
Copy link
Member

cramertj commented Sep 3, 2019

That would help, but IMO use futures::{future::TryFutureExt, stream::TryStreamExt}; is still pretty noisy and the future and stream prefixes seem to stutter.

@taiki-e
Copy link
Member Author

taiki-e commented Sep 5, 2019

I noticed that the current one is useful when using with macro.

use futures::{select, join, FutureExt, TryFutureExt};

Also, the removal of reexports of items other than traits that I originally wanted to do is included in #1858.
So, I'm going to close this PR.

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.

Remove top-level re-export of Poll?
3 participants