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

Add Required Features #45

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cwfitzgerald
Copy link

This improves ergonomics a little bit by adding the required-features flag on the tests that need it, so you don't get compile errors.

I also included the .vscode config to improve the default experience. I personally would like to see this committed as there's having the default config compile out of the box would be great.

@teoxoy
Copy link
Owner

teoxoy commented Jul 7, 2023

It's unfortunate that you can't force a feature for a specific test instead. required-features will skip them instead. I'm not sure what's worse, a compilation error if the feature is not passed or silently skipping the test...

Regarding the rust-analyzer config, I'm hesitant to add it to the repo since not everyone uses vscode and I always have it enabled and only turn it off for specific projects that have issues with it (which is rare since it's not good practice to have incompatible features).

I think it would be great if we could find a way to force a feature flag for a test or document that tests require the mint feature instead.

Let me know what you think.

@cwfitzgerald
Copy link
Author

force a feature flag for a test

Unfortunately this doesn't seem to be possible. Unfortunately you can't enable features on yourself in a dev dependencies. We could split out the integration tests into it's own package, which could force enable features.

@cwfitzgerald
Copy link
Author

We could split out the integration tests into it's own package, which could force enable features.

What do you think of this solution?

@teoxoy
Copy link
Owner

teoxoy commented Aug 19, 2023

I think it's the best solution we have right now - thanks for proposing it!

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.

None yet

2 participants