Skip to content

Build a html string from services/ember-cli#76

Merged
joostdevries merged 4 commits intomasterfrom
html-only
Jul 30, 2015
Merged

Build a html string from services/ember-cli#76
joostdevries merged 4 commits intomasterfrom
html-only

Conversation

@joostdevries
Copy link
Copy Markdown
Member

My idea is basically to have all of the build logic in services/ember-cli so that we don't have to do the same stuff in multiple places.. That way we can probably also unit-test services/ember-cli and have a pretty good baseline for overall functionality

@joostdevries
Copy link
Copy Markdown
Member Author

@Gaurav0 @rwjblue this further simplified the process, whatyathink?

@rwjblue
Copy link
Copy Markdown
Member

rwjblue commented Jul 28, 2015

👍 to the changes, would you mind adding a super simple unit test as a starting point for future testing initiatives?

@Gaurav0
Copy link
Copy Markdown
Contributor

Gaurav0 commented Jul 28, 2015

@joostdevries While definitely simpler, I want to ensure that we don't expose index.html to the user until we know we've ensured he can't break the app (other than the build of his own app) with it.

@joostdevries
Copy link
Copy Markdown
Member Author

@Gaurav0 at this point it's not exposed as I'm taking it directly out of blueprints instead of merging the blueprint into gist.files

@joostdevries
Copy link
Copy Markdown
Member Author

@rwjblue test :-)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

using actual models here overcomplicated things, so I chose to use normal objects.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@rwjblue
Copy link
Copy Markdown
Member

rwjblue commented Jul 30, 2015

👍 - Looks good, left one small comment...

joostdevries pushed a commit that referenced this pull request Jul 30, 2015
Build a html string from services/ember-cli
@joostdevries joostdevries merged commit 62631ab into master Jul 30, 2015
@joostdevries joostdevries deleted the html-only branch July 30, 2015 19:29
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