Skip to content
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

Prepare for upcoming breaking changes in FastBoot 1.0 #18

Merged
merged 2 commits into from
Apr 18, 2017

Conversation

simonihmig
Copy link
Owner

A few things will be gone with the upcoming FastBoot 1.0 release:

  • no more process.env.EMBER_CLI_FASTBOOT (as there is just one unified build step)
  • no initializer/(browser|fastboot)/* (again, just one build)

The changes here should continue to work with pre 1.0 and also support post 1.0 versions.

@simonihmig
Copy link
Owner Author

Btw, here is some context: ember-fastboot/ember-cli-fastboot#360

Not sure what's happening with Travis again, cannot install phantomjs?

Copy link

@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. I don't have context into what this addon is doing but I am surprised that it is calling fastboot-filter-initializers.

@@ -46,8 +43,9 @@ module.exports = {
},

config(env, baseConfig) {
if (!env)
if (!env) {

Choose a reason for hiding this comment

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

When is env not going to be defined?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Not sure tbh

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't really remember, why I introduce this guard. Maybe to eliminate any possibility of being the cause of a bug in early days. I will remove this in the next release.

@simonihmig
Copy link
Owner Author

The failing tests are due to yarnpkg/yarn#3138 😞

@andreasschacht
Copy link
Collaborator

@simonihmig should we force travis to use a later node version?

@simonihmig
Copy link
Owner Author

Yes, we could so, as a workaround, but should revert once the yarn bug is fixed!

@andreasschacht andreasschacht merged commit 1648ade into master Apr 18, 2017
This was referenced Apr 21, 2017
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.

3 participants