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

Testing with --no-default-features didnt seem to work in CI #319

Merged
merged 3 commits into from
May 20, 2024

Conversation

NyxCode
Copy link
Collaborator

@NyxCode NyxCode commented May 17, 2024

No description provided.

@NyxCode
Copy link
Collaborator Author

NyxCode commented May 17, 2024

@escritorio-gustavo Could you take a quick peek at this?

As far as I can tell the CI ran cargo t --no-default-features only in the root directory, where the --no-default-features argument does nothing. So, possibly, default-features = false has been broken for a while. Oops!

Biggest change I've made here is to remove almost all of the #[cfg(feature = "serde-compat")] gates - I only kept them where we actually need them (e.g for printing the warning - we don't have the termcolor dependency without serde-compat).

In the other places, I used if cfg!(feature = "serde-compat") instead. That makes a lot of things, especially imports, easier.
As you can see in the CI run for the first commit, those were most of the issues - crate::attr::Serde was imported, but it was gated behind the feature flag.

Does this seem alright to you?

@escritorio-gustavo
Copy link
Contributor

those were most of the issues - crate::attr::Serde was imported, but it was gated behind the feature flag.

Oops, sorry about that! Thankfully #290 hasn't been published yet as it was made after the release of version 8.1.0, so hopefully no one has been affected

Biggest change I've made here is to remove almost all of the #[cfg(feature = "serde-compat")] gates [...], I used if cfg!(feature = "serde-compat") instead.
[...]
Does this seem alright to you?

Sure! I'm fine with both

@escritorio-gustavo escritorio-gustavo merged commit badbac0 into main May 20, 2024
14 checks passed
@escritorio-gustavo escritorio-gustavo deleted the ci-issues branch May 20, 2024 11:36
@NyxCode
Copy link
Collaborator Author

NyxCode commented May 21, 2024

Ah, perfect! I thought you did those changes quite some time ago. Even better!

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