This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Document rationale for Dart VM flag prefix match #30195
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dart VM flags are passed to Flutter via an fml::CommandLine::Option that
looks something like:
{"dart-flags, "--max_profile_depth 1,--trace_service"}
We perform a prefix match to handle cases where Dart VM options take
arguments.
Adding the comment since in a recent review I found myself wondering why
we're using a prefix match to begin with. While the original author had
forgotten, the good news is, he wrote a test that covers this exact
case. This comment just removes one level of indirection for future
readers.
|
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. 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. |
bdero
approved these changes
Dec 7, 2021
bkonyi
approved these changes
Dec 7, 2021
cbracken
added
waiting for tree to go green
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 8, 2021
zanderso
pushed a commit
to flutter/flutter
that referenced
this pull request
Dec 8, 2021
* 9b74c21 Run iOS scenario apps on iPhone 11 and iOS 14 (flutter/engine#30104) * 4108898 Add error handler for os.rmtree calls (flutter/engine#30169) * 55c5e56 Don't run curl in verbose mode when --verbose set (flutter/engine#30170) * a1fe531 Win32: Implement DispatchAccessibilityAction (flutter/engine#30187) * a7a8024 Update buildroot to db6d037 (ToT). (flutter/engine#30171) * 7ccf290 Revert "Run iOS scenario apps on iPhone 11 and iOS 14 (#30104)" (flutter/engine#30193) * 1313e73 Add unconditional waits on fml::Semaphore. (flutter/engine#30165) * 094ff9e Roll Fuchsia Mac SDK from EAlr46NQ8... to zMg5gNi2E... (flutter/engine#30190) * a572ab5 Roll Fuchsia Linux SDK from WGMjaVH60... to s03VQc7lX... (flutter/engine#30191) * 07c6f41 Roll Skia from f333f5614a9b to 06f3d68627c2 (24 revisions) (flutter/engine#30192) * 8796f0f Update Swiftshader to dc0f131930 (flutter/engine#30188) * c1e235d Add length guard to fix Windows build with VS2019 16.11.7 headers (flutter/engine#30189) * 7831529 Roll Skia from 06f3d68627c2 to 1c4cf27965bd (2 revisions) (flutter/engine#30194) * 5672f9e Document rationale for Dart VM flag prefix match (flutter/engine#30195) * 1c10e68 Roll buildroot to 430b57c. (flutter/engine#30197) * e32d04a Roll Skia from 1c4cf27965bd to 543b8681c7f2 (1 revision) (flutter/engine#30198) * 6981c04 [ios platform view] fix overlay zPosition (flutter/engine#29930) * efaccf0 renew cirrus key (flutter/engine#30186) * 9b80c08 Roll Skia from 543b8681c7f2 to 00edeefab7f4 (1 revision) (flutter/engine#30200) * 164045d Roll Fuchsia Mac SDK from zMg5gNi2E... to QeaP059wu... (flutter/engine#30203) * 83b84d3 Roll Skia from 00edeefab7f4 to 21b8ccb7393c (3 revisions) (flutter/engine#30206)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 8, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dart VM flags are passed to Flutter via an fml::CommandLine::Option that
looks something like:
We perform a prefix match to handle cases where Dart VM options take
arguments.
Adding the comment since in a recent review I found myself wondering why
we're using a prefix match to begin with. While the original author had
forgotten, the good news is, he wrote a test that covers this exact
case. This comment just removes one level of indirection for future
readers.
No tests added since this is a comment-only change, and a test already
exists that covers this scenario.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.