-
Notifications
You must be signed in to change notification settings - Fork 5
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 Allowing warnings #80
Conversation
@@ -71,4 +71,4 @@ jobs: | |||
- uses: actions-rs/cargo@v1 | |||
with: | |||
command: clippy | |||
args: --all-targets --all-features -- -D warnings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is opening the door for all the warnings. Today have none due to this.
Are we sure we want this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alternative is to pin rustc
to 1.80
, the one that is marking the allowing of box_pointers
is rustc 1.81.0 (eeb90cda1 2024-09-04)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about updating src/envoy
path content? I am aware it is auto-generated from protobuf descriptors. So maybe change the source of truth or add some customization https://github.com/Kuadrant/wasm-shim/blob/main/build.rs#L55-L106
It is all about deleting
#![allow(box_pointers)]
That is a lint being removed in rust 1.80
Whichever route you all decide to take, I'd like for it to remain a temporary fix until this PR is released into a newer version of the crate... thanks 🙏 |
Good to see there is a fix. Indeed, we need just a temporary fix (and a reminder to undo when the upstream fix is available) Currently:
The fix we want is |
a4607dd
to
f1df905
Compare
Signed-off-by: dd di cesare <[email protected]>
f1df905
to
8629088
Compare
@eguzki Added a |
In order to fix https://github.com/Kuadrant/wasm-shim/actions/runs/10735988827/job/29774629002?pr=77