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

tests(iroh): Feature-flag the doc test #2983

Merged
merged 5 commits into from
Dec 2, 2024
Merged

tests(iroh): Feature-flag the doc test #2983

merged 5 commits into from
Dec 2, 2024

Conversation

flub
Copy link
Contributor

@flub flub commented Dec 2, 2024

Description

This test can only work if the feature is enabled. Feature-flag the
entire test contents.

Closes #2950.

Breaking Changes

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

This test can only work if the feature is enabled.  Feature-flag the
entire test contents.
@flub flub enabled auto-merge December 2, 2024 09:24
@flub flub added this to the v0.29.0 milestone Dec 2, 2024
@flub flub added docs Improvements or additions to documentation test labels Dec 2, 2024
Copy link

github-actions bot commented Dec 2, 2024

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2983/docs/iroh/

Last updated: 2024-12-02T10:55:52Z

@dignifiedquire
Copy link
Contributor

To also enable [LocalSwarmDiscovery], it can be added as another service in the

will still fail in cargo doc as the link is invalid with default features

Copy link

github-actions bot commented Dec 2, 2024

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: a1a6ec8

@flub
Copy link
Contributor Author

flub commented Dec 2, 2024

To also enable [LocalSwarmDiscovery], it can be added as another service in the

will still fail in cargo doc as the link is invalid with default features

Do we support cargo doc without all features? How does docs.rs run cargo doc?

Should we not make sure CI fails on these things?

@dignifiedquire
Copy link
Contributor

Do we support cargo doc without all features?

I think we should, because locally the initial thing you do is run cargo doc

Should we not make sure CI fails on these things?

we probably should

@flub
Copy link
Contributor Author

flub commented Dec 2, 2024

We have

[package.metadata.docs.rs]
all-features = true

So I think everything will work fine. Of course you can argue this is not a great local experience. But neither is having some of the docs accidentally hidden when building locally. Not sure what to do here...

@dignifiedquire
Copy link
Contributor

But neither is having some of the docs accidentally hidden when building locally

no need to hide things, just don't make it a link

flub added 4 commits December 2, 2024 11:25
- Checks docs with default features

- Checks doc tests compile with all feature combinations
#![cfg_attr(
not(feature = "discovery-local-network"),
doc = "`LocalSwarmDiscovery`"
)]
Copy link
Contributor

Choose a reason for hiding this comment

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

beautiful 🤮

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You asked for this 😝

@flub flub added this pull request to the merge queue Dec 2, 2024
Merged via the queue into main with commit b30f218 Dec 2, 2024
26 checks passed
@flub flub deleted the flub/feat-flag-doc-test branch December 2, 2024 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation test
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

iroh-net: default features are not compatible with doc tests
2 participants