-
Notifications
You must be signed in to change notification settings - Fork 76
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
Added serializer blueprint and updated docs #124
base: master
Are you sure you want to change the base?
Added serializer blueprint and updated docs #124
Conversation
Sorry I haven't had a chance to look at this PR, could someone with more experience with Ember blueprints look at this please? |
@mattmarcum I think a |
@fivetanley So you mean this PR would make more sense without the README modification? |
We ran into this recently with the ActiveModelAdapter (this reminds me I need to fix it there too). Basically, if you don't define a serializer for a model, it will look for |
As somebody who usually skims for the installation step and then starts hacking, I think making the application.js for a default serializer/adapter is the move that sets users up for success.
Yeah, if we add the files, that seems like a better solution to me. |
I was reluctant to make it the default application serialiser/adapter but your point that it would prompt if there’s a conflict is true… so yeah, I also support adding it to the blueprint. |
@mattmarcum, thanks for this PR. I think the generator is the right approach - we actually hit this issue ourselves. It looks like we just need to update the README to direct people to use the generator and this can be merged. |
@aexmachina Can you make the readme change or close the PR? |
This pr is a squashed version of #122
It adds a serializer blueprint and updates the docs.