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

Clippy warnings in CI #437

Closed
johanhelsing opened this issue Mar 24, 2024 · 0 comments · Fixed by #443
Closed

Clippy warnings in CI #437

johanhelsing opened this issue Mar 24, 2024 · 0 comments · Fixed by #443
Labels
ci Only relevant for CI

Comments

@johanhelsing
Copy link
Owner

johanhelsing commented Mar 24, 2024

After rust 1.77, we know get clippy warnings in ci:

https://github.com/johanhelsing/matchbox/actions/runs/8407545605/job/23023258077?pr=436

 error: field `1` is never read
  --> bevy_matchbox/src/socket.rs:73:67
   |
73 | pub struct MatchboxSocket<C: BuildablePlurality>(WebRtcSocket<C>, Box<dyn Debug + Send + Sync>);
   |            -------------- field in this struct                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `MatchboxSocket` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis
   = note: `-D dead-code` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(dead_code)]`
help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field
   |
73 | pub struct MatchboxSocket<C: BuildablePlurality>(WebRtcSocket<C>, ());
   |       

To repro:

rustup update
cargo clippy --all-targets --target wasm32-unknown-unknown -p bevy_matchbox -- -D warnings

Did not happen with rust 1.76

The field was added in 4295f6d by @garryod.

It looks like it's there to just make sure we don't drop the task handle. I'm not sure what the practical difference is between detaching and keeping the handle in the socket, though... Maybe @garryod remembers?

Perhaps we should just add #[allow(dead_code)] for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Only relevant for CI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant