-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[rust-server] Fix clippy warnings #13907
[rust-server] Fix clippy warnings #13907
Conversation
I have just published a built Docker image on rev 5de15fb (only for Arm64, sorry!). |
when testing it locally, I got the following errors in the rust-server sample:
can you please take a look when you've time? |
@wing328 I think there is no way to support both version of clippy for allowing or denying lints, so we need to decide to support the latest only or keep the current behaviour. How do you think about that? |
@wing328 I think we need to consider pinning the project to fixed version of clippy and Rust in general. Otherwise what will happen is our CI checks may fail in future even though the code hasn't changed. I mentioned this before here: We need to define a MSRV and have the CI install this fixed version |
I apologise I missed your comments in the previous pull request. I agree that we should set MSRV for supporting and testing. Since clippy is intended to use in project-local, there is no backward compatibility as shown as the testing result now. To keep the MSRV older (it means the warnings are continuously shown in later toolchains), or to keep later (older toolchains will not be supported) is an important decision I think. |
the suggestion sounds good. |
Any updates on this? We have an additional warnings x37 times since v1.67:
|
can you please PM me via slack next week to discuss how to move forward with this? have a nice weekend. |
tests passed via #14758. we'll merge and see if later someone can contribute a PR or two to improve the CI tests. |
and as a workaround, users can enable/disable the warnings: https://doc.rust-lang.org/nightly/clippy/usage.html |
This pull request fixes these clippy warnings:
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
(6.1.0) (minor release - breaking changes with fallbacks),7.0.x
(breaking changes without fallbacks)@frol @farcaller @richardwhiuk @paladinzh @jacob-pro