-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add .env file support for option --dart-define-from-file
#128668
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
Add .env file support for option --dart-define-from-file
#128668
Conversation
--dart-define-from-file
fc63a11 to
04300d7
Compare
04300d7 to
bd1a6f6
Compare
andrewkolos
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.
I haven't done a complete review of this, but I did add some comments to start out with.
8d1d979 to
7b0a822
Compare
2ee2118 to
bded33e
Compare
bded33e to
bbaa46e
Compare
andrewkolos
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.
Thanks for your continued work on this PR!
I think this is getting really close to a good state. Most of my remaining comments are nitpicks.
packages/flutter_tools/test/commands.shard/permeable/build_bundle_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/commands.shard/permeable/build_bundle_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/commands.shard/permeable/build_bundle_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/commands.shard/permeable/build_bundle_test.dart
Outdated
Show resolved
Hide resolved
bbaa46e to
46cf071
Compare
|
I think every of your suggestion has been applied to the code @andrewkolos. One step closer to the good state. |
46cf071 to
95030fd
Compare
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
🚀
christopherfujino
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.
RSLGTM
95030fd to
89ebd18
Compare
| /// | ||
| /// Returns a record of key and value as strings. | ||
| MapEntry<String, String> _parseProperty(String line) { | ||
| final RegExp blockRegExp = RegExp(r'^\s*([a-zA-Z_]+[a-zA-Z0-9_]*)\s*=\s*"""\s*(.*)$'); |
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.
All of these regular expressions are constants. They should be pulled out of this function so that they will not be reallocated on each iteration of the loop in convertEnvFileToJsonRaw.
flutter/flutter@35085c3...bc49cd1 2023-07-06 [email protected] Allow long-press gestures to continue even if buttons change. (flutter/flutter#127877) 2023-07-06 [email protected] Enable unreachable_from_main lint - it is stable now!!1 (flutter/flutter#129854) 2023-07-06 [email protected] Update labeler to new label names (flutter/flutter#130040) 2023-07-05 [email protected] MergeableMaterial: Fix adding a slice and separating it (flutter/flutter#128804) 2023-07-05 [email protected] Update infrastructure issue template for new priority scheme (flutter/flutter#129741) 2023-07-05 [email protected] Fix typo in canvas example (flutter/flutter#129879) 2023-07-05 [email protected] Reland Fix AnimatedList & AnimatedGrid doesn't apply MediaQuery padding #129556 (flutter/flutter#129860) 2023-07-05 [email protected] Change from "created via performance template" to "from: performance template" (flutter/flutter#130035) 2023-07-05 [email protected] Removes deprecated APIs from v2.6 in `binding.dart` and `widget_tester.dart` (flutter/flutter#129663) 2023-07-05 [email protected] Add new hot reload case string (flutter/flutter#130008) 2023-07-05 [email protected] Manual roll Flutter Engine from 987b621eac4e to bd2e42b203e1 (32 revisions) (flutter/flutter#130023) 2023-07-05 [email protected] Add simple unit tests for annotations.dart file (flutter/flutter#128902) 2023-07-05 [email protected] fix a bug when android uses CupertinoPageTransitionsBuilder... (flutter/flutter#114303) 2023-07-05 [email protected] Add .env file support for option `--dart-define-from-file` (flutter/flutter#128668) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages 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 Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
|
This pr should get a lot of attention. This is huge. |
Set `.env` regex list as constants. This pull request fixes the nit related in this comment: #128668 (comment)
Set `.env` regex list as constants. This pull request fixes the nit related in this comment: flutter#128668 (comment)
Proposal
I suggest to make possible to specify .env files to the --dart-define-from-file in addition to the Json format.
Issue
Close #128667
Pre-launch Checklist
///).