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

Changed the start-app test helper to use Ember.assign. #6193

Merged
merged 1 commit into from
Aug 19, 2016

Conversation

workmanw
Copy link
Contributor

Some discussion here: ember-cli/ember-new-output#5 . I mistakenly submitted the PR to that repo not knowing it was generated.

@Turbo87
Copy link
Member

Turbo87 commented Aug 18, 2016

@workmanw awesome, thanks!

@@ -5,8 +5,7 @@ import config from '../../config/environment';
export default function startApp(attrs) {
let application;

let attributes = Ember.merge({}, config.APP);
attributes = Ember.merge(attributes, attrs); // use defaults, but you can override;
let attributes = Ember.assign({}, config.APP, attrs); // use defaults, but you can override;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move the comment to the previous line for line length reasons.

@nathanhammond
Copy link
Contributor

LGTM with one modification.

@workmanw workmanw force-pushed the start-app-ember-assign branch from 9c6a302 to ff58fc0 Compare August 19, 2016 11:29
@workmanw
Copy link
Contributor Author

@nathanhammond 👍 Updated.

@workmanw workmanw force-pushed the start-app-ember-assign branch from ff58fc0 to 3689ee0 Compare August 19, 2016 11:54
@nathanhammond
Copy link
Contributor

@homu r+

@homu
Copy link
Contributor

homu commented Aug 19, 2016

📌 Commit 3689ee0 has been approved by nathanhammond

homu added a commit that referenced this pull request Aug 19, 2016
Changed the start-app test helper to use `Ember.assign`.

Some discussion here: ember-cli/ember-new-output#5 . I mistakenly submitted the PR to that repo not knowing it was generated.
@homu
Copy link
Contributor

homu commented Aug 19, 2016

⌛ Testing commit 3689ee0 with merge 9fe0573...

@homu
Copy link
Contributor

homu commented Aug 19, 2016

☀️ Test successful - status

@homu homu merged commit 3689ee0 into ember-cli:master Aug 19, 2016
@nathanhammond nathanhammond added this to the v2.9.0 milestone Aug 20, 2016
@Turbo87 Turbo87 mentioned this pull request Sep 16, 2016
homu added a commit that referenced this pull request Sep 16, 2016
Revert #6193

Optimistic change, didn't actually do any testing. RE: #6274
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants