-
Notifications
You must be signed in to change notification settings - Fork 6k
Guard impeller references in platform_view_android.cc for google build #33487
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 (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Can we use Google Testing to verify this change? |
|
@CaseyHillers can you point to instructions on how to do that? |
|
I started a TGP on cl/449791593, which contains this change. |
|
Arrrgh. It wasn't synced properly and now I'm trying to fix that. |
|
@CaseyHillers FWIW, engine contributors are not likely to go through the exercise of testing this. I don't know what we can do about avoiding this short of:
Running a test on this was pretty trivial since I had a bad CL I could quickly clone and patch with this change. If I was working on some random impeller or Android change I would not think to do that and it may be much more difficult. |
|
test-exempt: build configuration will be tested by target build environment |
|
@CaseyHillers there are a bunch of failures on my TGP but they all seem to be related to other problems with web or a different engine patch than the impeller one. |
|
By Google Testing, I meant the presubmit frob status. It looks like that doesn't run on the engine. |
@CaseyHillers @zanderso @chinmaygarde
For b/233045685