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

fix deprecated import of getOwner #1124

Merged
merged 1 commit into from
Dec 9, 2016
Merged

Conversation

sly7-7
Copy link
Contributor

@sly7-7 sly7-7 commented Dec 9, 2016

Should fix the second deprecation in #1122

Not sure if using setOwner this way in the tests is the right way though.
At least for the SessionService, maybe using the setupTest thing from is the way to go, but I guess it implies a bigger refactor

I just realize that it would be breaking change...

@@ -78,7 +78,6 @@
"ember-cli-import-polyfill": "^0.2.0",
"ember-cli-is-package-missing": "^1.0.0",
"ember-cookies": "^0.0.10",
"ember-getowner-polyfill": "^1.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

This is still needed but in a later version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed that because of ember-getowner-polyfill is not required for Ember 2.3.0 and later, please remove from your package.json`` at build time, but I think if you want to keep ember-simple-auth compatible with ember 1.12+ this dependency is required, right ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we'll want to maintain the current compatibility unless not possible anymore.


describe('InternalSession', () => {
describe('InternalSession', function() {
Copy link
Member

Choose a reason for hiding this comment

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

using arrow functions should be fine here unless you want to use this

@marcoow marcoow added this to the 1.2 milestone Dec 9, 2016
@marcoow marcoow merged commit 0376a12 into mainmatter:master Dec 9, 2016
@marcoow
Copy link
Member

marcoow commented Dec 9, 2016

🎉

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.

2 participants