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

Adds ember engines in "Application Concerns" #763

Merged
merged 13 commits into from
Apr 26, 2019
Merged

Adds ember engines in "Application Concerns" #763

merged 13 commits into from
Apr 26, 2019

Conversation

villander
Copy link
Contributor

It's solves the big problem around ember ecosystem #701 - this will increase our visibility and the actual proposal of ember engines in ember ecosystem and de-emphasize code-splitting/lazy-loading a bit.

cc: @MelSumner @locks @jenweber @acorncom

Copy link
Contributor

@jenweber jenweber left a comment

Choose a reason for hiding this comment

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

I added some suggested wording changes. Thanks for your work on this!

guides/release/applications/ember-engines.md Outdated Show resolved Hide resolved
guides/release/applications/ember-engines.md Outdated Show resolved Hide resolved
guides/release/applications/ember-engines.md Outdated Show resolved Hide resolved
guides/release/applications/ember-engines.md Outdated Show resolved Hide resolved
guides/release/applications/ember-engines.md Outdated Show resolved Hide resolved
guides/release/applications/ember-engines.md Outdated Show resolved Hide resolved
guides/release/applications/ember-engines.md Outdated Show resolved Hide resolved
guides/release/applications/ember-engines.md Outdated Show resolved Hide resolved
jenweber and others added 4 commits April 23, 2019 21:24
Co-Authored-By: villander <nousvillander@gmail.com>
Co-Authored-By: villander <nousvillander@gmail.com>
@villander
Copy link
Contributor Author

I applied all suggested changes, thanks so much @jenweber

@jenweber jenweber self-requested a review April 26, 2019 20:10
Copy link
Contributor

@jenweber jenweber left a comment

Choose a reason for hiding this comment

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

This is ready to go now!

I raised the question of file naming and TOC placement with the learning team a couple days ago. It seems that ember-engines makes sense as the file name/url because there is not really a more general concept to use here. It's about the name of the library, the same as how we renamed "Models" to "Ember Data."

There were no objections to the merge in general, since there was already good cautionary info in this PR.

@jenweber jenweber merged commit 9d882d3 into ember-learn:master Apr 26, 2019
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.

2 participants