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

Doc updates to prepare for FastBoot 1.0 #70

Merged
merged 3 commits into from
Jun 8, 2017

Conversation

kratiahuja
Copy link
Contributor

@kratiahuja kratiahuja commented May 22, 2017

Fixes #54

This PR fixes issues on the website which called to use ember fastboot. With FastBoot 1.0 and starting with the RC builds, ember fastboot will be gone. Also tweaked wording a bit to explain the new build in short.

The irony this app is using ember fastboot --serve-assets instead of ember serve. Going to bump ember version and using ember serve in a seperate PR.

cc: @stefanpenner @tomdale

@kratiahuja kratiahuja changed the title Prepare for FastBoot 1.0 Doc updates to prepare for FastBoot 1.0 May 22, 2017
This PR fixes issues on the website which called to use `ember fastboot`. With FastBoot 1.0 and starting with the RC builds, `ember fastboot` will be gone. Also tweaked wording a bit to explain the new build in short.
@@ -764,8 +748,8 @@ The server offers an [Express middleware][express] that can be integrated into a

```javascript
var server = new FastBootServer({
appFile: appFile,
vendorFile: vendorFile,
appFiles: [appFile, appFile-fastboot],
Copy link
Contributor

@stefanpenner stefanpenner May 22, 2017

Choose a reason for hiding this comment

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

appFile-fastboot is likely not correct, as it is not a valid variable name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohoo yeah, I meant appFastBootFile to denote the additional asset for overrides

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stefanpenner fixed.

@kratiahuja
Copy link
Contributor Author

ember-cli upgrade is in this PR : #72

@kratiahuja
Copy link
Contributor Author

@stefanpenner fixed the naming a bit. can you please take a look again when you have some time?

@stefanpenner
Copy link
Contributor

@tomdale I think your eyes on this is important.

@stefanpenner
Copy link
Contributor

cc @kristoferbaxter

@kratiahuja
Copy link
Contributor Author

@tomdale pinging again to see if you had a chance to review this PR?

@kristoferbaxter
Copy link

Apologies for the delay. This is first on my list for tomorrow.

when you deploy an Ember app, the FastBoot server also loads these two
files.
FastBoot works by running a singular build as it was running without the FastBoot
addon being installed. It basically loads the same assets `app.js` and `vendor.js`

Choose a reason for hiding this comment

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

Perhaps this could be simplified a bit.

FastBoot works by running a singular build of your application, leveraging the common `app.js` and `vendor.js` files on the server and browser clients. However, on the server an additional asset `app-fastboot.js` will leveraged. This asset contains FastBoot specific overrides defined by the application or other addons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds much better :). Just tweaked the "server and browser clients" to call "browser and Node" since everywhere else we refer it as Node.

The FastBoot addon sets the `EMBER_CLI_FASTBOOT` environment variable
when it is building the FastBoot version of the application. Use
this to prevent the inclusion of a library in the FastBoot build:
The FastBoot addon simply provides the FastBoot server a manifest of

Choose a reason for hiding this comment

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

Could 'simply' be removed from this sentence without changing the overall meaning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@kratiahuja
Copy link
Contributor Author

@kristoferbaxter Thanks for the feedback and apologies for the delay here. Updated per your suggestion. Could you please take another look?

@kristoferbaxter
Copy link

LGTM!

@kratiahuja
Copy link
Contributor Author

@stefanpenner any objections to merging this? If not, I'll merge this and we will be one step closer to 1.0 :)

@stefanpenner
Copy link
Contributor

@kristoferbaxter thanks for the review!

@stefanpenner stefanpenner merged commit 7dbaa97 into ember-fastboot:master Jun 8, 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.

Remove ember-fastboot from docs
3 participants