Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: Fix test warning #155

Merged
merged 1 commit into from
Oct 28, 2024
Merged

Conversation

gretchenfrage
Copy link
Contributor

On current master (30f38c8d8728ee99a2194bc7a2c5f556cb6a0a3a) with Rust 1.82 (rustc 1.82.0 (f6e511eec 2024-10-15), cargo 1.82.0 (8f40fc59f 2024-08-21)), running cargo check --all-features --tests emits the following warning:

warning: associated items `new` and `rewind` are never used
  --> src/common/rewind.rs:20:19
   |
18 | impl<T> Rewind<T> {
   | ----------------- associated items in this implementation
19 |     #[cfg(test)]
20 |     pub(crate) fn new(io: T) -> Self {
   |                   ^^^
...
36 |     pub(crate) fn rewind(&mut self, bs: Bytes) {
   |                   ^^^^^^
   |
   = note: `#[warn(dead_code)]` on by default

warning: `hyper-util` (lib test) generated 1 warning

This PR fixes that and also does some other, opportunistic cleanup in the same file:

  • Removes unused new and rewind methods
  • Changes the #[allow(dead_code)] annotation on new_buffered to more specific feature-gating
  • Removes commented out code which hasn't been touched in a long time

Note on the second point: from searching through the project for the string new_buffered, it is called solely from hyper-util/src/server/conn/auto/mod.rs on lines 144, 150, and 285.

  • The first two call sites are annotated with #[cfg(feature = "http1")] and #[cfg(feature = "http2")] respectively
  • The server::conn::auto module itself is feature-gated with #[cfg(any(feature = "http1", feature = "http2"))]
  • The server module is of course feature-gated with #[cfg(feature = "server")].

I confirmed that in this branch that the following commands succeed without warnings:

cargo check
cargo check --all-features
cargo check --all-features --tests
cargo check --features http1
cargo check --features http2
cargo check --features http1,http2
cargo check --features http1,http2,server
cargo test --all-features

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup!

It's a copy from the type in hyper, where several of those methods were used, and clearly we don't need them here.

@seanmonstar seanmonstar merged commit 05b13f4 into hyperium:master Oct 28, 2024
16 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants