Skip to content

Conversation

@SparshithNR
Copy link

@SparshithNR SparshithNR commented Feb 20, 2020

This PR address the review comments from #729

@SparshithNR SparshithNR changed the title [WIP]Embroider stuff Changes required for Embroider build to support Fastboot apps Feb 20, 2020
@dnalagatla
Copy link
Contributor

dnalagatla commented Feb 21, 2020

Thank you @SparshithNR for working on updating code based on new review comments. The change looks good to me.

Copy link
Contributor

@kratiahuja kratiahuja left a comment

Choose a reason for hiding this comment

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

LGTM overall, except the concern on wanting to understand the data-fastboot-ignore usecase more.

sparshithNR and others added 2 commits February 24, 2020 14:41
Updating README so it is for user.
Copy link
Contributor

@kratiahuja kratiahuja left a comment

Choose a reason for hiding this comment

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

Will merge once CI is completed.

@ef4
Copy link
Contributor

ef4 commented Mar 24, 2020

This is still an important issue, but after much more end-to-end testing of full apps with fastboot & embroider I don't actually think this is the right solution.

Instead of mangling the build output in postBuild -- a step I can't really guarantee will always even exists -- the core code in this PR belongs in https://github.com/ember-fastboot/fastboot instead, where it can discover the right assets when the Fastboot instance is reading from distPath.

Also, the earlier reverted version of this feature is still present in the latest release because no release has been cut since it was reverted.

@kiwiupover
Copy link
Member

@dnalagatla @ef4 and @rwjblue what needs to happen to this PR before we can cut an embroider version of ember-cli-fastboot

@ef4
Copy link
Contributor

ef4 commented Jun 16, 2020

This PR is superseded, the equivalent changes were already made in fastboot itself.

We probably do need a release of ember-cli-fastboot that upgrades to that newer fastboot.

There is also work to do here to make ember-cli-fastboot natively emit the new format accepted by fastboot, although that is not a blocker because fastboot accepts all older formats too.

@rwjblue rwjblue closed this Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants