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

Prefer doc(no_inline) to doc(hidden) for reexports #2479

Merged
merged 1 commit into from Aug 22, 2021

Conversation

nipunn1313
Copy link
Contributor

Fixes #2472

@nipunn1313
Copy link
Contributor Author

The cargo clippy failure appears to be a regression introduced in nightly 2021-08-17. Afaict it's a false positive from clippy.

The failure of cargo +1.41.0 is interesting since it's clearly ok on some stables around it. It's also clearly a non-regression (claiming that things aren't documented there). What do you suggest we do here to make CI happy? Could conditionally ignore the warning with something like https://github.com/dtolnay/rustversion - or unconditionally ignore the warning.

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks! Could you change to use no_inline only on trait reexports? *_internal macros and async_await module are not public APIs.

@taiki-e
Copy link
Member

taiki-e commented Aug 22, 2021

Could you do the same for the trait reexports in futures?

#[doc(hidden)]
pub use futures_core::future::{Future, TryFuture};
#[doc(hidden)]
pub use futures_util::future::{FutureExt, TryFutureExt};
#[doc(hidden)]
pub use futures_core::stream::{Stream, TryStream};
#[doc(hidden)]
pub use futures_util::stream::{StreamExt, TryStreamExt};
#[doc(hidden)]
pub use futures_sink::Sink;
#[doc(hidden)]
pub use futures_util::sink::SinkExt;
#[cfg(feature = "std")]
#[doc(hidden)]
pub use futures_io::{AsyncBufRead, AsyncRead, AsyncSeek, AsyncWrite};
#[cfg(feature = "std")]
#[doc(hidden)]
pub use futures_util::{AsyncBufReadExt, AsyncReadExt, AsyncSeekExt, AsyncWriteExt};

@nipunn1313
Copy link
Contributor Author

yep - thanks for catching! I missed that file in my audit! (I had used vim -p $(rg "doc.*hidden") which capped to an upper bound number of files by mistake). I double checked and confirmed I covered everything after hitting up futures/src/lib.rs

Thanks!

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks!

@taiki-e taiki-e merged commit 8be0a59 into rust-lang:master Aug 22, 2021
@taiki-e taiki-e added 0.3-backport: pending The maintainer accepted to backport this to the 0.3 branch, but backport has not been done yet. docs labels Aug 22, 2021
taiki-e pushed a commit that referenced this pull request Aug 28, 2021
@taiki-e taiki-e mentioned this pull request Aug 28, 2021
@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 Aug 28, 2021
taiki-e pushed a commit that referenced this pull request Aug 30, 2021
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.

doc(hidden) reexports
2 participants