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

Adds some documentation for JSONApiSerializer #3531

Merged
merged 1 commit into from
Jul 27, 2015
Merged

Conversation

simaob
Copy link
Contributor

@simaob simaob commented Jul 11, 2015

Following the conversation on #3455, this Pull Request adds some documentation for the JSONApiSerializer, and two of its methods keyForAttribute and keyForRelationship.

This documentation is heavily inspired in the existing documentation of the JSONSerializer, as suggested in the mentioned issue.

I hope it is meets what is expected. Let me know if you think it needs more work.

gamesPlayed: DS.attr(),
club: DS.belongsTo('club')
});
App.Club = DS.Model.extend({
Copy link
Member

Choose a reason for hiding this comment

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

Lets use Ember CLI style examples for these models.

@simaob
Copy link
Contributor Author

simaob commented Jul 17, 2015

cheers, will go over your comments.

players: DS.hasMany('player')
});
```
```js
Copy link
Member

Choose a reason for hiding this comment

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

For some reason code samples don't render correctly unless they have an empty line above the first ```.

Copy link
Contributor

Choose a reason for hiding this comment

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

@simaob Can you add an empty line between the code examples?

@simaob
Copy link
Contributor Author

simaob commented Jul 17, 2015

Made changes and pushed.

`JSONAPISerializer` supports the http://jsonapi.org/ spec and is the
serializer recommended by Ember Data.

This serializer normalizes a JSONApi payload that looks like:
Copy link
Contributor

Choose a reason for hiding this comment

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

JSONApi => JSON API

@wecc
Copy link
Contributor

wecc commented Jul 18, 2015

Added some comments about minor things. When that's fixed I'm 👍 to merge

@simaob
Copy link
Contributor Author

simaob commented Jul 18, 2015

@wecc made the changes requested. Thanks for the review and sorry for some clumsiness with the capitalization!

attribute values and serializing relationships.

`JSONAPISerializer` supports the http://jsonapi.org/ spec and is the
serializer recommended by Ember Data.
Copy link
Contributor

Choose a reason for hiding this comment

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

Lines 93-99 are duplicates of 14-20, could be removed.

@wecc
Copy link
Contributor

wecc commented Jul 19, 2015

@simaob I discovered one more thing but other than that looks good. Thank you for your patience!

@wecc
Copy link
Contributor

wecc commented Jul 19, 2015

Can you also squash the commits and prefix the commit message with [DOC release]?

@simaob
Copy link
Contributor Author

simaob commented Jul 20, 2015

Should be all set. :) Hope I did it correctly :P

bmac added a commit that referenced this pull request Jul 27, 2015
Adds some documentation for JSONApiSerializer
@bmac bmac merged commit 9a1649c into emberjs:master Jul 27, 2015
@bmac
Copy link
Member

bmac commented Jul 27, 2015

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants