-
-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
02d508e
to
7dab4e3
Compare
Thank you for cleaning this up (and getting it over the finish line). Looking good. |
@Turbo87 the travis failures may be related to ember-cli/ember-cli-blueprint-test-helpers#33 |
haven't had time to investigate yet, but somethings wrong. it works on the emberjs PR, but I must've missed something here. |
ef4ce31
to
378fdb7
Compare
@trabus it seems that indeed I have the same issue as in ember-cli/ember-cli-blueprint-test-helpers#33 |
@rwjblue any idea how to avoid setting |
@Turbo87 I would recommend that you remove ember-data from this lib, and add the blueprints directly. The intent of this addon was to provide a snapshot of the blueprints in ember-cli at the state where we diverged, providing backward-compatibility support for consumers that are not using addonified ember and ember-data versions. |
I tried removing the ember-data dependency but somehow Travis still failed with the same problem |
6d2447b
to
5bdca89
Compare
@stefanpenner @rwjblue @trabus I think this is ready for review and merging now |
We don't need those because this addon only contains blueprints which are tested via mocha/node
import Ember from 'ember'; | ||
import { initialize } from '<%= dasherizedModulePrefix %>/instance-initializers/<%= dasherizedModuleName %>'; | ||
import { module, test } from 'qunit'; | ||
import destroyApp from '../../helpers/destroy-app'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trabus should these be absolute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that those are the exact same files that are now merged into Ember.js too
this looks good, i left one question. Then we can merge |
see emberjs/ember.js#13029
resolves #3
resolves #2