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

Remove polyfillMemoryLeakPrevention #336

Merged
merged 1 commit into from
Sep 27, 2018

Conversation

tmquinn
Copy link
Contributor

@tmquinn tmquinn commented Sep 17, 2018

As of qunit@2.6.0, this polyfill is no longer necessary and can cause issues
in apps the implement ember-qunit

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Looks good, can you also update yarn.lock?

@rwjblue
Copy link
Member

rwjblue commented Sep 17, 2018

CI is failing due to Node 4 being dropped by ember-data@2.18.5 😩. I don't think we need ember-data here so we should probably remove it and confirm that it fixes the CI issue.

@tmquinn
Copy link
Contributor Author

tmquinn commented Sep 18, 2018

D'oh! Done.

@choheekim
Copy link

@tmquinn @rwjblue even after ember-data's removed the CI still fail due to that ember-data is being used in test code, tests/unit/legacy-2-x/module-for-model-test.js.

@rwjblue
Copy link
Member

rwjblue commented Sep 26, 2018

CI should be unblocked by #338

@rwjblue
Copy link
Member

rwjblue commented Sep 26, 2018

@tmquinn - Can you drop the last commit (we ended up needing ember-data) and rebase against master? Should be good to merge after that...

@tmquinn tmquinn force-pushed the remove-polyfillMemoryLeakPrevention branch 3 times, most recently from 1517e8f to a7f078c Compare September 27, 2018 03:39
As of qunit@2.6.0, this polyfill is no longer necessary and can cause issues
in apps the implement ember-qunit
@tmquinn tmquinn force-pushed the remove-polyfillMemoryLeakPrevention branch from a7f078c to c23c06b Compare September 27, 2018 03:47
@rwjblue rwjblue merged commit 653323e into emberjs:master Sep 27, 2018
@rwjblue
Copy link
Member

rwjblue commented Sep 27, 2018

Thank you @tmquinn!

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.

3 participants