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

Fix warnings about unexpected cfg(feature) values #2968

Merged
merged 4 commits into from
May 21, 2024

Conversation

svix-jplatte
Copy link
Contributor

Motivation

CI is failing due to a new rustc lint. More about this lint in the recent blog post.

Solution

See individual commits. I chose different solutions for different parts of the codebase where warnings were emitted.

The cfg for it was impossible to enable as of
tokio-rs#441.
Previously, enabling this optional dependency did not do anything.
No need to use a custom cfg to run off-by-default tests.
It might be possible to make it work with tower_make instead, but that
crate looks to be unmaintained (no longer present in the tower repo
which is specified as the repository URL) so it should likely just be
removed as a dependency.
@svix-jplatte svix-jplatte requested review from hawkw, davidbarsky and a team as code owners May 13, 2024 16:41
@svix-jplatte
Copy link
Contributor Author

@hds This should fix all the CI failures from #2925 as all the netlify CI jobs succeeded.

@@ -71,33 +71,8 @@ where
}
}

#[cfg(feature = "tower-util")]
Copy link

Choose a reason for hiding this comment

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

These could be migrated to tower_make instead, see #2971

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, see the commit description for why I didn't do that.

Copy link
Member

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

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

thanks!

@svix-jplatte
Copy link
Contributor Author

@davidbarsky any reason this wasn't merged after approval?

@davidbarsky
Copy link
Member

@davidbarsky any reason this wasn't merged after approval?

No reason other than "Phabricator taught me muscle memory that the PR author can land PRs/diffs after approval" 😅. Landing this.

@davidbarsky davidbarsky merged commit d61b039 into tokio-rs:master May 21, 2024
55 checks passed
@svix-jplatte svix-jplatte deleted the jplatte/cfg branch May 21, 2024 15:01
@hds hds mentioned this pull request Nov 19, 2024
20 tasks
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.

3 participants