Skip to content

Conversation

@PiotrFLEURY
Copy link
Contributor

@PiotrFLEURY PiotrFLEURY commented Jun 11, 2023

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

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 11, 2023
@PiotrFLEURY PiotrFLEURY changed the title Add .env file support for option Add .env file support for option --dart-define-from-file Jun 11, 2023
@PiotrFLEURY PiotrFLEURY force-pushed the flutter_tools/dot-env-dart-define-from-file-proposal branch 2 times, most recently from fc63a11 to 04300d7 Compare June 17, 2023 15:35
@PiotrFLEURY PiotrFLEURY force-pushed the flutter_tools/dot-env-dart-define-from-file-proposal branch from 04300d7 to bd1a6f6 Compare June 20, 2023 17:19
Copy link
Contributor

@andrewkolos andrewkolos left a 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.

@PiotrFLEURY PiotrFLEURY force-pushed the flutter_tools/dot-env-dart-define-from-file-proposal branch 3 times, most recently from 8d1d979 to 7b0a822 Compare June 21, 2023 14:58
@PiotrFLEURY PiotrFLEURY force-pushed the flutter_tools/dot-env-dart-define-from-file-proposal branch 6 times, most recently from 2ee2118 to bded33e Compare June 28, 2023 15:22
@github-actions github-actions bot removed the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 28, 2023
@PiotrFLEURY PiotrFLEURY force-pushed the flutter_tools/dot-env-dart-define-from-file-proposal branch from bded33e to bbaa46e Compare June 28, 2023 16:43
Copy link
Contributor

@andrewkolos andrewkolos left a 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.

@PiotrFLEURY PiotrFLEURY force-pushed the flutter_tools/dot-env-dart-define-from-file-proposal branch from bbaa46e to 46cf071 Compare June 29, 2023 11:51
@PiotrFLEURY
Copy link
Contributor Author

I think every of your suggestion has been applied to the code @andrewkolos.

One step closer to the good state.

@PiotrFLEURY PiotrFLEURY force-pushed the flutter_tools/dot-env-dart-define-from-file-proposal branch from 46cf071 to 95030fd Compare June 29, 2023 15:58
Copy link
Contributor

@andrewkolos andrewkolos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

🚀

@andrewkolos andrewkolos requested a review from eliasyishak June 29, 2023 19:15
@andrewkolos andrewkolos added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 29, 2023
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

@PiotrFLEURY PiotrFLEURY force-pushed the flutter_tools/dot-env-dart-define-from-file-proposal branch from 95030fd to 89ebd18 Compare July 5, 2023 11:58
@christopherfujino christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 5, 2023
@auto-submit auto-submit bot merged commit 168d807 into flutter:master Jul 5, 2023
///
/// 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*(.*)$');
Copy link
Member

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.

@PiotrFLEURY PiotrFLEURY mentioned this pull request Jul 6, 2023
8 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 6, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jul 6, 2023
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
@debkanchan
Copy link

This pr should get a lot of attention. This is huge.

auto-submit bot pushed a commit that referenced this pull request Jul 13, 2023
Set `.env` regex list as constants.

This pull request fixes the nit related in this comment: #128668 (comment)
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
Set `.env` regex list as constants.

This pull request fixes the nit related in this comment: flutter#128668 (comment)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add .env format file support for --dart-define-from-file option

6 participants