-
-
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
Add examples to the Serializer API docs #4440
Conversation
24d3837
to
945ef9d
Compare
It's injected as a service. | ||
It can be used to push records from a non flat data structure server | ||
response. | ||
The `store` property is the application's `store` that contains |
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.
so it's a bit confusing here because you have the store passed into the function's parameter list, and also this.store
, which gets set when the serializer is instantiated.
I think this documentation is for this.store
because of @property
, but isn't used in the example below.
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 call.
945ef9d
to
7b37a76
Compare
}, | ||
}); | ||
``` | ||
|
||
@method serialize | ||
@param {DS.Model} record |
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.
Can you update this to be DS.Snapshot
?
Nice writeup! 💯 👍 |
7b37a76
to
49dee7f
Compare
@pangratz @fivetanley This pr has been updated. |
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.
LGTM other than a couple of minor things
It can be used to push records from a non flat data structure server | ||
response. | ||
The `store` property is the application's `store` that contains | ||
all records. It can be used to look up serializer for other model |
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.
Extreme Nitpicking™️ – double space here :)
Also look up serializer for other model type
=> look up serializers for other model types
?
@method serialize | ||
@param {DS.Model} record | ||
@param {DS.Snapshot} record |
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.
Should this parameter be named snapshot
instead of record
?
Sorry @wecc, GitHub didn't show up your comment when I was merging. Wanna open up a follow up PR? |
Planck time trolled us! 🕐 |
No description provided.