Repository navigation
[macOS] Remove warning when running with merged threads. - #185673
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. 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. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
github-actions
Bot
added
engine
knopp
added
CICD
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the FlutterEngine to log a deprecation warning when running with unmerged UI and platform threads. A critical syntax error was identified in the Objective-C code where a multi-line string literal lacks the required '@' prefix on the second line, which would cause a compilation error and violates the Google Objective-C Style Guide.
Piinks
added
engine
bd11d92 to
69f59d5
Compare
Not sure. I don't think there's a straightforward way to read NSLog output for current process. It does usually output to stderr and we might be able to redirect that, but it's not guaranteed. I don't think this is worth it though given that this warning message will probably be rather short-lived as we plan to remove the opt-out anyway. |
69f59d5 to
43fc18e
Compare
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
43fc18e to
1e7e086
Compare
|
I rebased the PR as I suspect the golden diffs were because this was branched off a stale commit. |
loic-sharma
added
CICD
flutter-dashboard
Bot
removed
the
autosubmit
The engine has been running with merged threads by default probably fora year now, this message is obsolete.
List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.