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

check std feature against kv_unstable #434

Merged
merged 3 commits into from Jan 8, 2021
Merged

check std feature against kv_unstable #434

merged 3 commits into from Jan 8, 2021

Conversation

KodrAus
Copy link
Collaborator

@KodrAus KodrAus commented Jan 8, 2021

Closes #433

There's an incorrect #[cfg(feature = "std")] in kv::value that's broken users upgrading to 0.4.12. This PR fixes that up and runs a few more permutations in CI.

KodrAus added 2 commits Jan 8, 2021
Check a few more permutations of `kv_unstable` features
@KodrAus
Copy link
Collaborator Author

@KodrAus KodrAus commented Jan 8, 2021

It looks like this slipped through because the feature mismatch was masked by dev dependencies.

@Byron
Copy link

@Byron Byron commented Jan 8, 2021

Thanks a lot for the quick fix! I was just investigating why smol fails to compile (and probably many more).

@KodrAus
Copy link
Collaborator Author

@KodrAus KodrAus commented Jan 8, 2021

Once CI goes through I'll publish a 0.4.13 version. In the meantime 0.4.12 has been yanked.

@KodrAus KodrAus merged commit b169072 into master Jan 8, 2021
22 checks passed
@KodrAus KodrAus deleted the fix/kv-features branch Jan 8, 2021
@Byron
Copy link

@Byron Byron commented Jan 8, 2021

Thanks so much - another cargo update later and the world is fixed :).

@Byron
Copy link

@Byron Byron commented Jan 8, 2021

Thinking about it - in case you believe this could possibly happen again, then cargo all-features might be the tool to enforce testing all permutations automatically.

@KodrAus
Copy link
Collaborator Author

@KodrAus KodrAus commented Jan 8, 2021

It's definitely something worth looking into! We do something like this over in env_logger. One of the challenges here is that the dependencies whose features we were misusing also appear as dev-dependencies, which is why permutations of feature flags in tests didn't pick this up. So now we check them in nightly using the unstable avoid-dev-deps flag.

@Byron
Copy link

@Byron Byron commented Jan 8, 2021

Even though I don't truly follow 😅 I would like to point out that the tool can be configured with options to handle special cases better. It seems this feature was entirely contributed, so I would hope that other needs that log might have could be contributed there as well.

@KodrAus
Copy link
Collaborator Author

@KodrAus KodrAus commented Jan 8, 2021

Ah great I'll take a look! If cargo-check-all-features passes through our additional -Z avoid-dev-deps args then that should be everything we need.

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.

2 participants