Skip to content

Conversation

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Oct 17, 2017

@rwjblue rwjblue requested a review from Turbo87 October 17, 2017 19:03
Robert Jackson added 2 commits October 17, 2017 15:37
PR is submitted to fix the plugin, once merged and released we can drop
this back down.
The new API relies on the the public API added for `Ember.getOwner` /
`Ember.setOwner`.
export { default as moduleFor } from './legacy-2-x/module-for';
export {
default as moduleForComponent,
} from './legacy-2-x/module-for-component';
Copy link
Member

Choose a reason for hiding this comment

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

this looks like we might want the 100 chars/line here too

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, done in a later commit.

import Ember from 'ember';
import QUnit from 'qunit';
import { QUnitAdapter } from 'ember-qunit';
import QUnitAdapter from './adapter';
Copy link
Member

Choose a reason for hiding this comment

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

how did that ever work?! 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

NO CLUE!?!?!?

teardownContext,
setupRenderingContext,
teardownRenderingContext,
} from 'ember-test-helpers';
Copy link
Member

Choose a reason for hiding this comment

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

duplicate import?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, no. The other one is an export { .... } from which does not bring the exported things into the current scope.

Copy link
Member

Choose a reason for hiding this comment

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

oh, right...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants