Skip to content

Reapply flutter_assets - #13567

Merged
szakarias merged 2 commits into
flutter:masterfrom
szakarias:reapplyFlutterAssets
Dec 14, 2017
Merged

Reapply flutter_assets#13567
szakarias merged 2 commits into
flutter:masterfrom
szakarias:reapplyFlutterAssets

Conversation

@szakarias

@szakarias szakarias commented Dec 14, 2017

Copy link
Copy Markdown
Contributor

This reverts commit ee65db1 except the commit marking complex_layout_ios__start_up as flaky.

The problem with the Windows bots should be fixed with #13564.

@szakarias

szakarias commented Dec 14, 2017

Copy link
Copy Markdown
Contributor Author

cc @yjbanov .

jakobr-google
jakobr-google approved these changes Dec 14, 2017
View reviewed changes

@jakobr-google jakobr-google left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

RSLGTM

@yjbanov

yjbanov commented Dec 14, 2017

Copy link
Copy Markdown
Contributor

lgtm

@szakarias
szakarias merged commit 5e18c07 into flutter:master Dec 14, 2017
@xster

xster commented Dec 15, 2017

Copy link
Copy Markdown
Member

Should this add some gitignores in the templates and the example projects?

@Hixie

Hixie commented Dec 15, 2017

Copy link
Copy Markdown
Contributor

This dramatically increased the size of the gallery. Is that expected? Are we including assets twice now or some such?

@Hixie Hixie mentioned this pull request Dec 15, 2017
Closed
@mehmetf

mehmetf commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

Could we revert this and give @tvolkert a chance to review? It affects Google build system in a significant way. Rolls are blocked because of it.

@tvolkert

tvolkert commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

@mehmetf not easily because we'd have to revert several engine commits as well.

s'ok - I'll roll this once I'm back - there shouldn't be that much activity over the holidays anyway.

@tvolkert

tvolkert commented Dec 19, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

@szakarias was this tested with --project=assets? That flag is used for prebuilt applications (which are used inside Google).

flutter/packages/flutter_tools/lib/src/commands/run.dart

Line 115 in 12f4055

argParser.addOption('project-assets',

flutter/packages/flutter_tools/lib/src/resident_runner.dart

Line 406 in 12f4055

_assetBundle = new AssetBundle.fixed(_projectRootPath, projectAssets);

@szakarias

szakarias commented Dec 19, 2017

Copy link
Copy Markdown
Contributor Author

No, I will do that.

@alanrussian

alanrussian commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

I need to merge #13521 into Google but this is currently blocking me, so I have to manually merge in #13521. Would be nice to have this fixed ASAP. Thanks!

@tvolkert

tvolkert commented Dec 20, 2017

Copy link
Copy Markdown
Contributor

FTR @szakarias the error we're seeing inside Google patching this in is:

I/flutter ( 3392): [INFO:engine.cc(586)] Could not configure asset bundle at path:

You can reproduce by patching in cl/179637015

DaveShuckerow pushed a commit to DaveShuckerow/flutter that referenced this pull request May 14, 2018
@szakarias
Reapply flutter_assets (flutter#13567)
cb30105 
* Revert "Revert engine (flutter#13547)"

This reverts commit ee65db1.

* Keep flaky mark
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@yjbanov yjbanov yjbanov approved these changes
@mravn-google mravn-google Awaiting requested review from mravn-google
+1 more reviewer
@jakobr-google jakobr-google jakobr-google approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@szakarias @yjbanov @xster @Hixie @mehmetf @tvolkert @alanrussian @jakobr-google @googlebot