Fix warnings

[filnet]

• Tested on all platforms changed
• Compilation warnings were addressed
• cargo fmt has been run on this branch
• cargo doc builds successfully
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

[filnet]

Would it make sense to fail the builds if there are warnings like the ones this PR fixes ?

[madsmtm]

Thanks for this, these have been bothering me for a while!

Would it make sense to fail the builds if there are warnings like the ones this PR fixes ?

I would say yes (can easily be done with RUSTFLAGS: "-D warnings")

[filnet]

I would say yes (can easily be done with RUSTFLAGS: "-D warnings")

Where would one do that. There are no CI related files in the winit repo.

[maroider]

There are no CI related files in the winit repo.

GitHub actions looks in .github/workflows for workflows, and our CI is in .github/workflows/ci.yml.

[filnet]

The wasm builds fail as expected.

[filnet]

I am getting spammed by CI messages about unrelated failed checks whenever I push changes to the branch.
Doesn't make for a good user experience when contributing to winit.

The failed checks should be disabled until properly fixed ?

[filnet]

GitHub actions looks in .github/workflows for workflows, and our CI is in .github/workflows/ci.yml.

Totally overlooked them ! Thanks.

[madsmtm]

Please don't disable the failing nightly checks; they're very useful for spotting future incompatibility errors (see #2078), and they don't prevent people from merging the PR. If we disable them, we forget them 😉

[maroider]

and they don't prevent people from merging the PR

They don't‽

Now that you've mentioned it, the "Squash and merge" button does look awfully clickable.

I am getting spammed by CI messages about unrelated failed checks whenever I push changes to the branch.
Doesn't make for a good user experience when contributing to winit.

The notification spam is unfortunate, but I don't know if we can turn those off.

[madsmtm]

The notification spam is unfortunate, but I don't know if we can turn those off.

I don't think we can, though people can control it themselves under Settings => Notifications => Actions.

[filnet]

Ping.

[rukai]

Ah, just a heads up that warnings are not stable.
So if you want to fail on warnings you should really pin your rust version, see https://github.com/rukai/mado_rust_library_ci_template/ for more details.

[madsmtm]

Well, we run CI on nightly, which would surface changes to warnings that would start failing our CI in a way that prevents a merge.

I'm not strictly opposed to pinning our Rust version, though I think it should be for other reasons like guaranteeing a MSRV.

[rukai]

Ah sure, if your paying enough attention to the nightly workflow then that would work too.