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
Fix lint errors in vulkan directory. #19991
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
dda4917
to
2d11b07
Compare
vulkan/vulkan_window.cc
Outdated
// This is just to make lint happy: MakeVulkan isn't defined unless SK_VULKAN is | ||
// defined, and lint doesn't define SK_VULKAN. | ||
#ifdef SK_VULKAN |
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.
Can we just use NOLINT? I'd prefer that.
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.
So would I, but no, without those #ifdef
s, it's not just a lint error, it's a syntax error, since MakeVulkan
isn't defined at all if SK_VULKAN
isn't.
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.
LGTM modulo the ifdef. I'd prefer not linting that file if nolint can't be used. There is a broader issue we need to fix with that guy somehow.
OK, I reverted |
f0165ec
to
fd6087e
Compare
The Fuchsia build failure was a flake in a test that is now disabled. Is this ready to land? |
@gspencergoog is OOO but my understanding is that this was ready. We can wait for him to come back. |
Description
This fixes lint errors in the vulkan directory. This might be a little more controversial, but the reason that the lints were failing here was because of an include path problem that I think I solved.
There's one heinous hack in here. It's totally safe, but see if you can spot it...