-
-
Notifications
You must be signed in to change notification settings - Fork 498
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
chore: Fix clippy lints #2569
chore: Fix clippy lints #2569
Conversation
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.
Don't use ovr
prefix because that was used for OculusVR in the old Oculus desktop SDK. Instead call the header openvr_driver_shim.h
or something similar
Sure, how is that code built on windows btw? MSVC or clang? Because if it's msvc I'd need to add some extra pragmas. I kinda forgot that something as awful as msvc even exists when I did that... |
@The-personified-devil MSVC is used. clang is only used to generate C headers from rust code (bindgen) |
9f265ee
to
baf141e
Compare
I can't get any warnings from msvc no matter what I do (even tho cc is adding a lot of warning flags), so I'll just leave it at that when it comes to the c++ part, if anyone finds msvc screaming at them I'll take a look and go fix it. |
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.
Seems fair. Code looks good to me
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.
Sorry, there are still 3 warnings on windows when running cargo clippy
That's because of the not windows marked function. And the way I understood you was that we'd have to update wgpu, which we can't. So I'd rather leave those three lints than do some gigantic hack with cfg attrs for this. |
I don't know when we'll upgrade. we are blocked on egui which seems to have a slow release cycle.you can just do |
For the imports, instead move the |
baf141e
to
2fa9cce
Compare
This should preferrably be merged as a rebase, because all of the commits kinda do their own thing.
Also I only put allows and not expects for the vulkan layer cuz that's gonna go anyway soon ™️
I also didn't yet go through all the allows and replace them with expects