-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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 ian@hixie.ch Allow long-press gestures to continue even if buttons change. (flutter/flutter#127877) 2023-07-06 goderbauer@google.com Enable unreachable_from_main lint - it is stable now!!1 (flutter/flutter#129854) 2023-07-06 ian@hixie.ch Update labeler to new label names (flutter/flutter#130040) 2023-07-05 47639380+Snonky@users.noreply.github.com MergeableMaterial: Fix adding a slice and separating it (flutter/flutter#128804) 2023-07-05 ian@hixie.ch Update infrastructure issue template for new priority scheme (flutter/flutter#129741) 2023-07-05 737941+loic-sharma@users.noreply.github.com Fix typo in canvas example (flutter/flutter#129879) 2023-07-05 hans.muller@gmail.com Reland Fix AnimatedList & AnimatedGrid doesn't apply MediaQuery padding #129556 (flutter/flutter#129860) 2023-07-05 ian@hixie.ch Change from "created via performance template" to "from: performance template" (flutter/flutter#130035) 2023-07-05 109253501+pdblasi-google@users.noreply.github.com Removes deprecated APIs from v2.6 in `binding.dart` and `widget_tester.dart` (flutter/flutter#129663) 2023-07-05 helinx@google.com Add new hot reload case string (flutter/flutter#130008) 2023-07-05 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 987b621eac4e to bd2e42b203e1 (32 revisions) (flutter/flutter#130023) 2023-07-05 69246223+moylanm@users.noreply.github.com Add simple unit tests for annotations.dart file (flutter/flutter#128902) 2023-07-05 gipcjs@gmail.com fix a bug when android uses CupertinoPageTransitionsBuilder... (flutter/flutter#114303) 2023-07-05 piotr.fleury@gmail.com 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 bmparr@google.com,rmistry@google.com,stuartmorgan@google.com 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
///).