-
Notifications
You must be signed in to change notification settings - Fork 6k
Support running sound null safe kernels from flutter_jit_runner #50002
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 "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use 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. |
|
@CaseyHillers is there any way to try this patch in g3 before landing it? |
Yes, we can follow go/flutter-life-of-a-pr#google-testing-failed-how-do-i-debug once all the GitHub checks are green. |
81fd6f3 to
cd23bd3
Compare
|
All checks are green now and I can do this tomorrow. Are Fuchsia artifacts for pending PRs also fetched into google3 now? |
Yes, but you may need to patch in cl/600934274 for the short term. That will fix the issue where the wrong hash is being used on engine PR rolls. 🤞 it's submitted tomorrow. |
cd23bd3 to
9942b6a
Compare
9942b6a to
4eab829
Compare
|
I followed go/flutter-on-fuchsia-build#building-the-engine-for-smart-display to bring fuchsia artifacts into g3, everything works as intended (I can use sound kernels with flutter jit runner now) and couldn't find any breakages |
jrwang
left a comment
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
|
Thanks! I don't have permissions to merge, could you please help?
Jonny Wang ***@***.***> schrieb am Di., 30. Jan. 2024, 18:16:
… ***@***.**** approved this pull request.
LGTM
—
Reply to this email directly, view it on GitHub
<#50002 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADVTHBIC2ZVUNI2WN5JN53YRETHHAVCNFSM6AAAAABCI7332WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJRHEYDSNZXGY>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
…142564) flutter/engine@d20ed24...e6e1d6b 2024-01-30 [email protected] Roll Dart SDK from 734aae9604e8 to 488e33cd39de (1 revision) (flutter/engine#50185) 2024-01-30 [email protected] [Impeller] Reland: add interface for submitting multiple command buffers at once. (flutter/engine#50180) 2024-01-30 [email protected] Support running sound null safe kernels from flutter_jit_runner (flutter/engine#50002) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
--no-sound-null-safetyflags from native code and ninja argssound_null_safetyattribute toflutter_componentin ninja, which propagates todart_kernelPre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.