Make a kReleaseMode constant that is public. #27502

gspencergoog · 2019-02-04T18:39:11Z

Before this, we had several places where an isReleaseMode was defined, all with the same definition. This just makes it more broadly visible to allow our users to use it, as well as creating debug and profile versions, and adding a device lab test for it.

Since this is a const value, this makes it possible for a developer to easily mark blocks that can be removed at AOT compile time.

packages/flutter/lib/src/foundation/debug.dart

rmacnak-google · 2019-02-04T19:31:08Z

packages/flutter/lib/src/foundation/debug.dart

if (false) { dead(); } isn't tree shaking.

Are you saying that in your example the code is not being removed? (and if so, why isn't it?)

Or are you saying that my use of "tree shaking" is just the incorrect term for removing blocks like that at compile time?

Also, this:

static const Foo foo = Foo(debugName: true ? null : 'Name');

would not include the string 'Name' in the compiled output, right?

packages/flutter/lib/src/foundation/constants.dart

packages/flutter_tools/lib/src/build_info.dart

jonahwilliams · 2019-02-05T18:30:11Z

We need a way to test that these work properly

gspencergoog · 2019-02-05T21:25:50Z

Yeah... Do we currently have a test that builds all the different configurations (all five modes, apk and ios, and simulator/emulator)? I don't know of one. Not sure I want to be the first, but it would be helpful to have even without these variables needing testing.

jonahwilliams · 2019-02-05T21:27:48Z

If we're providing a contract like "guarantee to remove code in x-mode" we need to test it.

gspencergoog · 2019-02-05T21:52:39Z

Yeah, I agree. Kinda daunting though.

gspencergoog · 2019-02-06T00:09:08Z

Actually, it wasn't too bad. I have a devicelab test that will run in all five modes. Now I just need to fix the fact that debug and both dynamic modes don't seem to pick up the -D definitions on the frontend compiler's command line.

jonahwilliams · 2019-02-06T00:12:26Z

Maybe something in here does it? https://github.com/flutter/flutter/blob/master/packages/flutter_tools/gradle/flutter.gradle

jonahwilliams · 2019-02-06T00:12:36Z

Pass or not pass the flag, that is

gspencergoog · 2019-02-06T00:39:33Z

Good idea @jonahwilliams, but no, that doesn't make any difference, unfortunately.

gspencergoog · 2019-02-06T03:01:18Z

OK, I give up on making kDebugMode and kProfileMode work properly.

I'm just going to define kReleaseMode in terms of the existing dart.vm.product define, and remove the debug and profile ones. I kept the test, and it tests to make sure that kReleaseMode does what it should. The truth is, kReleaseMode is the one that is the most useful anyhow: you can use assert if you want to do something in debug mode, and profile mode can use the "profile()" method.

Not great, but a little cleaner than it was...

…(but doesn't).

goderbauer

LGTM

goderbauer · 2019-02-06T17:57:24Z

packages/flutter/lib/src/foundation/binding.dart

-  /// if (!const bool.fromEnvironment('dart.vm.product')) {
-  //   // Register your service extension here.
-  // }
+  /// void myRegistrationFunction() {


could we include the foundation/constant.dart import in this example to make clear where the constant is coming from?

Well, this is already in the foundation library (which is where the constant is defined). I added a comment in the example to indicate where it is defined instead, since I'm worried that the sample analyzer will complain if I import it again.

jonahwilliams

LGTM

Before this, we had several places where an isReleaseMode was defined, all with the same definition. This just makes it more broadly visible to allow our users to use it, as well as creating debug and profile versions, and adding a device lab test for it. Since this is a const value, this makes it possible for a developer to easily mark blocks that can be removed at AOT compile time.

googlebot added the cla: yes label Feb 4, 2019

gspencergoog requested a review from goderbauer February 4, 2019 18:39

goderbauer reviewed Feb 4, 2019

View reviewed changes

packages/flutter/lib/src/foundation/debug.dart Outdated Show resolved Hide resolved

goderbauer reviewed Feb 4, 2019

View reviewed changes

packages/flutter/lib/src/foundation/debug.dart Outdated Show resolved Hide resolved

rmacnak-google reviewed Feb 4, 2019

View reviewed changes

gspencergoog force-pushed the release_mode branch 6 times, most recently from a6a5de7 to 3e30af5 Compare February 5, 2019 17:41

gspencergoog changed the title ~~Make a kReleaseMode constant that is public.~~ Make a kReleaseMode, kDebugMode, kProfileMode constants that are public. Feb 5, 2019

gspencergoog force-pushed the release_mode branch from 3e30af5 to 8def903 Compare February 5, 2019 17:45

goderbauer added the framework flutter/packages/flutter repository. See also f: labels. label Feb 5, 2019

goderbauer reviewed Feb 5, 2019

View reviewed changes

packages/flutter/lib/src/foundation/constants.dart Outdated Show resolved Hide resolved

jonahwilliams reviewed Feb 5, 2019

View reviewed changes

packages/flutter_tools/lib/src/build_info.dart Outdated Show resolved Hide resolved

gspencergoog force-pushed the release_mode branch from 8def903 to 4384fb2 Compare February 5, 2019 21:23

gspencergoog changed the title ~~Make a kReleaseMode, kDebugMode, kProfileMode constants that are public.~~ Make kReleaseMode, kDebugMode, and kProfileMode constants that are public. Feb 6, 2019

gspencergoog force-pushed the release_mode branch 4 times, most recently from a6a398b to aabe836 Compare February 6, 2019 02:56

gspencergoog changed the title ~~Make kReleaseMode, kDebugMode, and kProfileMode constants that are public.~~ Make a kReleaseMode constant that is public. Feb 6, 2019

gspencergoog force-pushed the release_mode branch 2 times, most recently from b3325f1 to eff0974 Compare February 6, 2019 16:26

gspencergoog added 5 commits February 6, 2019 09:41

Make a kReleaseMode constant that is public.

0b791ce

Add env vars for debug, release, profile

1f3dcfa

Added devicelab test, and changed debug constant to what should work …

7f990ac

…(but doesn't).

Fix example, try adding more arguments.

45c6686

Remove debug and profile constants

4e64f08

gspencergoog force-pushed the release_mode branch from eff0974 to 4e64f08 Compare February 6, 2019 17:47

goderbauer approved these changes Feb 6, 2019

View reviewed changes

jonahwilliams approved these changes Feb 6, 2019

View reviewed changes

Review changes

561feab

gspencergoog merged commit da27f62 into flutter:master Feb 6, 2019

goderbauer mentioned this pull request Feb 7, 2019

Document how to check if profile/release/debug mode in dart #11392

Closed

gspencergoog mentioned this pull request Feb 7, 2019

Adding support for logical and physical key events #27627

Merged

gspencergoog deleted the release_mode branch February 12, 2019 17:36

jmagman mentioned this pull request Jul 21, 2021

Remove unused build_mode_test #86808

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2021

Make a kReleaseMode constant that is public. #27502

Make a kReleaseMode constant that is public. #27502

Uh oh!

Conversation

gspencergoog commented Feb 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rmacnak-google Feb 4, 2019

Choose a reason for hiding this comment

Uh oh!

gspencergoog Feb 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jonahwilliams commented Feb 5, 2019

Uh oh!

gspencergoog commented Feb 5, 2019

Uh oh!

jonahwilliams commented Feb 5, 2019

Uh oh!

gspencergoog commented Feb 5, 2019

Uh oh!

gspencergoog commented Feb 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams commented Feb 6, 2019

Uh oh!

jonahwilliams commented Feb 6, 2019

Uh oh!

gspencergoog commented Feb 6, 2019

Uh oh!

gspencergoog commented Feb 6, 2019

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

goderbauer Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

gspencergoog Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gspencergoog commented Feb 4, 2019 •

edited

Loading

gspencergoog Feb 4, 2019 •

edited

Loading

gspencergoog commented Feb 6, 2019 •

edited

Loading