joshka commented 2 months ago

Objective

Why did you make this PR?

I noticed that the feature flags were not documented and added these changes so the library documentation is kept perpetually updated.

Use document-features crate to automatically add documentation about the features to the crate documentation at the crate level.
Use the doc_auto_cfg feature to automatically add documentation to every item that requires a feature flag to be enabled. This is behind a docsrs feature flag as it can only be enabled using the nightly toolchain and this allows the documentation to be built with the stable toolchain when the feature is not enabled. See https://doc.rust-lang.org/rustdoc/unstable-features.html#doc_auto_cfg-automatically-generate-doccfg for more information.
Automatically scrape example code into the documentation using the unstable rustdoc-scrape-examples feature. See https://doc.rust-lang.org/nightly/rustdoc/scraped-examples.html for more information.

To test this change, run:

RUSTDOCFLAGS="--cfg docsrs" \
  cargo +nightly doc \
    -Zunstable-options -Zrustdoc-scrape-examples \
    --all-features --no-deps --open

Context

Discuss any context that may be needed for reviewers to understand the changes you've made. This may include related issues, previous discussion, or links to documentation or code.

New docs rendered on crate page:

Feature flags noted on item lists

Feature flags noted on items

Scraped examples

Feedback wanted

The command to generate the docs with all features enabled can't be added to the .cargo/config.toml as an alias because custom rustdoc flags and the nightly toolchain are needed. In other projects I've added this to a cargo-make file, a just file, or a bacon config file as a job. A small script similar to the existing ones could also be added to run this perhaps if needed (though cargo-xtask might be a reasonable approach to this more generically). Or perhaps just a shell script somewhere would also work as a shortcut. I'm not sure what's idiomatic for taffy devs on this sort of thing.

nicoburns commented 2 months ago

This looks pretty good to me (although I would appreciate another review as this is not something I am that familiar with). My one comment is that our "Documentation" CI check probably ought to be modified to run this new documentation generation command (or alternatively it should be added as an additional check).

We don't really have a task runner other than cargo atm. So I think between a CI check and the comment in the Cargo.toml, that's probably good enough for now.

joshka commented 2 months ago

This looks pretty good to me (although I would appreciate another review as this is not something I am that familiar with). My one comment is that our "Documentation" CI check probably ought to be modified to run this new documentation generation command (or alternatively it should be added as an additional check).

We don't really have a task runner other than cargo atm. So I think between a CI check and the comment in the Cargo.toml, that's probably good enough for now.

I considered both adding a new check and modifying the existing check and settled on modifying. The only real benefit to having two checks is that it means that you'll catch situations where generating with stable has a failure but nightly doesn't (or vice versa). This should generally be rare enough that it can be handled if it ever comes up rather than preemptively.

Changed in 93f58656 (with a couple of small wording tweaks to help explain this a little better)