-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Reland "Expose build mode in environment of asset transformer processes" #144958
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
Reland "Expose build mode in environment of asset transformer processes" #144958
Conversation
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
|
Not a big deal but for the future, please bear in mind this part of the Tree Hygiene doc:
Otherwise when doing code archeology later (e.g. walking through git blame) it gets quite confusing because the commit that actually stuck doesn't have the actual information that's useful. |
|
auto label is removed for flutter/flutter/144958, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
6bb5296 to
60d2bd3
Compare
|
@andrewkolos is this ready to be submitted? |
flutter/flutter@1a905d5...140edb9 2024-04-22 [email protected] Fixed few typos (flutter/flutter#147087) 2024-04-22 [email protected] Add Amir Panahandeh to AUTHORS (flutter/flutter#147052) 2024-04-22 [email protected] print traces when transforming an asset (flutter/flutter#146374) 2024-04-22 [email protected] Roll Packages from 88a3a56 to 01a32c4 (5 revisions) (flutter/flutter#147164) 2024-04-22 [email protected] Reland "Expose build mode in environment of asset transformer processes" (flutter/flutter#144958) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…es" (flutter#144958) Relands flutter#144752, which had to be reverted because the branch was stale. The original branch branched off `master` before flutter#144734 landed. That PR introduced a new `AssetTransformer` call site. This PR branch is identical to the original but with a new commit that addresses the new call site, [update new call sites](flutter@6bb5296).
flutter/flutter@1a905d5...140edb9 2024-04-22 [email protected] Fixed few typos (flutter/flutter#147087) 2024-04-22 [email protected] Add Amir Panahandeh to AUTHORS (flutter/flutter#147052) 2024-04-22 [email protected] print traces when transforming an asset (flutter/flutter#146374) 2024-04-22 [email protected] Roll Packages from 88a3a56 to 01a32c4 (5 revisions) (flutter/flutter#147164) 2024-04-22 [email protected] Reland "Expose build mode in environment of asset transformer processes" (flutter/flutter#144958) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…11510) Documents the feature long-ago implemented by flutter/flutter#144958. **Summary**: this documents that asset-transforming Dart packages can read `FLUTTER_BUILD_MODE` from the environment to determine what mode the app is being built in. This also tries to fix what I think was a typo in the original docs. If you are curious about what asset transformation is, read [Assets and Images (flutter.dev)](https://docs.flutter.dev/ui/assets/assets-and-images) to learn how to bundle assets (arbitrary files, such as images) into your app and then read [Asset transformation (flutter.dev)](https://docs.flutter.dev/ui/assets/asset-transformation). ## Presubmit checklist - [x] This PR is marked as draft with an explanation if not meant to land until a future stable release. - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer.
In service of #143348
When invoking a package to transform an asset, we set FLUTTER_BUILD_MODE to the CLI name of the build mode being used. Inspired by #101077 (comment):
Note for the reviewer: the interesting part of this change can be found in the commit set environment variable to build mode when running asset transformer…. The rest of the change is updating call sites with a new argument.
Boring stuff: Reland of #144752, which had to be reverted because the branch was stale. The original branch branched off
masterbefore #144734 landed. That PR introduced a newAssetTransformercall site.This PR branch is identical to the original but with a new commit that addresses the new call site, update new call sites.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.