-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Change setupStore helper default serializer to JSONAPISerializer #4754 #5003
Conversation
tests/integration/filter-test.js
Outdated
data: { | ||
id: snapshot.id, | ||
type: model.modelName, | ||
attributes: Ember.merge(Ember.merge({}, snapshot._attributes)) |
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.
Do we still need to specify Ember.merge
here twice if we aren't merging the id
?
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.
Good point ;-)
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.
❤️ this looks great.
@bmac This touches a lot of things, but is a nice cleanup. Any objections to merging? |
Sound good to me @hjdivad. |
Thanks for your contribution @eshtadc! |
…rjs#4754 (emberjs#5003) * Change setupStore helper default serializer to JSONAPISerializer emberjs#4754 * Update test per review feedback
…rjs#4754 (emberjs#5003) * Change setupStore helper default serializer to JSONAPISerializer emberjs#4754 * Update test per review feedback
@eshtadc awesome!! 🎉 🚀 |
Is this a breaking change? It seems like it could be, but perhaps I'm missing something. |
…ion when possible. This reverts commit 16b8a04. — The `deleteRecord() { return null; }` changes are to allow the back ported tests to work with the older JSONSerializer rather then the JSONAPISerializer which the test suite (in beta and master) use. For reference the JSONSerializer -> JSONAPISerializer landed -> emberjs#5003
…ion when possible. This reverts commit 16b8a04. — The `deleteRecord() { return null; }` changes are to allow the back ported tests to work with the older JSONSerializer rather then the JSONAPISerializer which the test suite (in beta and master) use. For reference the JSONSerializer -> JSONAPISerializer landed -> emberjs#5003
@workmanw I don't think so, as it merely changed the tests to use the more modern and recommended serializer. But if you notice a regression, please share! |
@workmanw keep us honest! |
No description provided.