Skip to content

Only compile doc tests that require non-default features if those features are enabled.#546

Open
ximon18 wants to merge 1 commit intomainfrom
fix-broken-doc-tests-with-default-features
Open

Only compile doc tests that require non-default features if those features are enabled.#546
ximon18 wants to merge 1 commit intomainfrom
fix-broken-doc-tests-with-default-features

Conversation

@ximon18
Copy link
Copy Markdown
Member

@ximon18 ximon18 commented Jun 25, 2025

@ximon18 ximon18 requested a review from a team June 25, 2025 10:21
@bal-e
Copy link
Copy Markdown
Contributor

bal-e commented Jun 25, 2025

I'm not sure this is the best way to do it, because this will remove examples from the documentation entirely, increasing confusion. This wouldn't be an issue for docs.rs, but the use case of building documentation locally (with limited feature flags) is important to me. Perhaps these examples should just be moved to the common module, where no cfg magic will be necessary.

Comment thread src/crypto/mod.rs
//! println!("verify result: {res:?}");
//! ```
#![cfg_attr(any(feature = "ring", feature = "openssl"), doc = r#"
```no_run
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could do a cfg_attr that only changes this line into:

```ignore

which will still include the code in the docs for all selected feature sets.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess ignore has the disadvantage that there will be no warning if the example gets out of sync with the code in domain.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It will run if the features are enabled and we do have CI runs that do that, so I think it is fine?

Copy link
Copy Markdown
Contributor

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

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

See comment

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.

5 participants