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

Use #[cfg(doc)] instead of docs.rs-specific cfg flag #287

Merged
merged 4 commits into from
Aug 18, 2021

Conversation

phil-opp
Copy link
Member

@phil-opp phil-opp commented Aug 1, 2021

The documentation can also be built locally using cargo doc --open. Doc links should be working there as well.

Cargo.toml Outdated
Comment on lines 45 to 47
[package.metadata.docs.rs]
rustdoc-args = ["--cfg", "docsrs"]

Copy link
Contributor

Choose a reason for hiding this comment

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

We still need to pass --cfg docsrs when building for docs.rs. This is so we can enable the doc_cfg feature, allowing us to note which features are required for which functions. For example:

With docsrs:
with

Without docsrs:
without

Copy link
Contributor

Choose a reason for hiding this comment

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

I just decided to add a doc_cfg nightly-only feature to handle this. Now running cargo doc just does the right thing by default.

Comment on lines +68 to +72
- name: "Run cargo doc"
uses: actions-rs/cargo@v1
with:
command: doc

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should do 3 runs of cargo doc here:

  1. cargo doc
    • default flags, on nightly
  2. RUSTDOCFLAGS="--cfg docsrs" cargo doc
    • flags docs.rs will use, on nightly
  3. cargo +stable doc --no-default-features --features=external_asm,instructions
    • make sure we can build the docs with the stable compiler

Copy link
Contributor

Choose a reason for hiding this comment

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

I decided to just have (1) and (3), now that we don't have the docsrs stuff.

@josephlr josephlr force-pushed the fix-cargo-doc-warnings branch 2 times, most recently from 58202ef to e1e5f50 Compare August 14, 2021 00:53
@josephlr josephlr force-pushed the fix-cargo-doc-warnings branch 4 times, most recently from 33b3c2c to a80a0cf Compare August 17, 2021 21:46
@josephlr josephlr force-pushed the fix-cargo-doc-warnings branch 2 times, most recently from 4048bab to b3db805 Compare August 18, 2021 09:09
phil-opp and others added 4 commits August 18, 2021 02:09
The documentation can also be built locally using `cargo doc --open`. Doc links should be working there as well.
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr josephlr force-pushed the fix-cargo-doc-warnings branch from b3db805 to 5354b02 Compare August 18, 2021 09:09
@josephlr josephlr merged commit a9cbf14 into master Aug 18, 2021
@josephlr josephlr deleted the fix-cargo-doc-warnings branch August 18, 2021 09:19
@phil-opp
Copy link
Member Author

Great, thanks a lot for your improvements!

@phil-opp phil-opp mentioned this pull request Sep 4, 2021
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