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

Rename doc_cfg to docsrs and use doc_auto_cfg #1450

Merged
merged 1 commit into from
May 10, 2024
Merged

Rename doc_cfg to docsrs and use doc_auto_cfg #1450

merged 1 commit into from
May 10, 2024

Conversation

newpavlov
Copy link
Member

@newpavlov newpavlov commented May 9, 2024

Renaming the configuration option also allows to enable back the unexpected_cfgs lint.

I've checked generated docs and it does not look like using doc_auto_cfg instead of explicit doc_cfgs causes any issues. The check was by no means exhaustive, but most cases look perfectly good.

@newpavlov newpavlov requested a review from dhardy May 9, 2024 21:56
///
/// [`TryCryptoRng`]: crate::TryCryptoRng
/// [`TryRngCore`]: crate::TryRngCore
/// [`CryptoRng`]: crate::CryptoRng
Copy link
Member Author

Choose a reason for hiding this comment

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

I've also fixed these doc item links. We probably should setup a CI job to check doc links in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

We do have an existing check-doc job, which should also detect bad in-repo links. Possibly we also need to deny warnings?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably, yes.

#[cfg_attr(feature = "serde1", derive(Serialize, Deserialize))]
#[cfg_attr(
feature = "serde1",
serde(bound(serialize = "W: Serialize, W::Sampler: Serialize"))
)]
#[cfg_attr(
feature = "serde1 ",
feature = "serde1",
Copy link
Member Author

@newpavlov newpavlov May 9, 2024

Choose a reason for hiding this comment

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

This is a typo caught by the unexpected_cfgs lint.

@@ -17,7 +17,7 @@ rust-version = "1.61"
include = ["src/", "LICENSE-*", "README.md", "CHANGELOG.md", "COPYRIGHT"]

[package.metadata.docs.rs]
rustdoc-args = ["--generate-link-to-definition"]
rustdoc-args = ["--cfg docsrs", "--generate-link-to-definition"]
Copy link
Member Author

Choose a reason for hiding this comment

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

Configuration option previously was not enabled for rand_distr.

Cargo.toml Outdated
all-features = true
rustdoc-args = ["--cfg", "doc_cfg", "-Zunstable-options", "--generate-link-to-definition"]
rustdoc-args = ["--cfg", "docsrs", "-Zunstable-options", "--generate-link-to-definition"]
Copy link
Member Author

@newpavlov newpavlov May 9, 2024

Choose a reason for hiding this comment

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

Do we still need -Zunstable-options?

Copy link
Member

Choose a reason for hiding this comment

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

IIRC it was there for --generate-link-to-definition

Copy link
Member Author

@newpavlov newpavlov May 10, 2024

Choose a reason for hiding this comment

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

We then should add it to rand_core and rand_distr as well. Although, I am not sure if this feature even works right now. The link in #1327 does not contain any links to definitions anymore.

UPD: Ah, according to this comment this flag is no longer needed. I've generated rand docs locally and source links were properly generated.

@newpavlov newpavlov merged commit 890ad8b into master May 10, 2024
14 checks passed
@newpavlov newpavlov deleted the docsrs branch May 10, 2024 10:46
@dhardy dhardy mentioned this pull request Nov 18, 2024
1 task
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.

2 participants