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

Better feedback on feature combinations #263

Open
danieleades opened this issue Aug 25, 2024 · 6 comments
Open

Better feedback on feature combinations #263

danieleades opened this issue Aug 25, 2024 · 6 comments

Comments

@danieleades
Copy link
Contributor

danieleades commented Aug 25, 2024

Would be great to have some documentation on valid combinations of features. This repo makes heavy use of features and some of these are mutually exclusive or have other relationships between them.

These should be documented and invalid combinations should have clear panic messages.

Would also be good to review that all of the valid combinations are tested in CI


i note that Cargo.toml contains these comments:

# NOTE: Only one of 'embedded' and 'embedded-hal-02' features can be enabled.
# Use "embedded' feature to enable embedded-hal=1.0 (embedded-io and embedded-io-async is part of embedded-hal).
# Use 'embedded-hal-0.2' feature to enable deprecated embedded-hal=0.2.3 (some hals is not supports embedded-hal=1.0 yet).

additionally, the following feature combinations fail to compile (not exhaustive):

  • --no-default-features
  • --no-default-features --features std
  • --no-default-features --features ardupilotmega

the following combinations do compile (not exhaustive):

  • --no-default-features --features std,ardupilotmega
@danieleades
Copy link
Contributor Author

Should also consider setting a custom default rust-analyzer command that doesn't emit every single target

@pv42
Copy link
Contributor

pv42 commented Aug 25, 2024

To also improve the documentation using #![feature(doc_auto_cfg)] to add those "Available on crate feature featurename only." badges would be a good idea.
Apparently this is a nightly feature but docs.rs is on nightly so #![cfg_attr(docsrs, feature(doc_auto_cfg))] would work.

@danieleades
Copy link
Contributor Author

@patrickelectric can you provide any context on what combinations of features are allowed?

@patrickelectric
Copy link
Member

This was probably broken on the pass PRs, some work need to be done to fix that besides adding tests in CI.

@danieleades
Copy link
Contributor Author

This was probably broken on the pass PRs, some work need to be done to fix that besides adding tests in CI.

I guess i'm trying to understand what is expected to work

@patrickelectric
Copy link
Member

This was probably broken on the pass PRs, some work need to be done to fix that besides adding tests in CI.

I guess i'm trying to understand what is expected to work

At the moment, check the combinations in CI. For conflict configuration is still necessary to document.

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

No branches or pull requests

3 participants