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

Update Documentation on Serializers and Rendering #2104

Merged
merged 1 commit into from
May 1, 2017
Merged

Update Documentation on Serializers and Rendering #2104

merged 1 commit into from
May 1, 2017

Conversation

cassidycodes
Copy link
Contributor

Purpose

Complete documentation where the text 'PR Please :)' was.

Changes

  • adds documentation to rendering.md
  • adds documentation to serializers.md

Caveats

I wasn't sure what the best practice was with a few of the methods on rendering.md. The documentation for these options seems to exist elsewhere, so I just linked to it.

Related GitHub issues

Additional helpful information

@mention-bot
Copy link

@cassidycodes, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bf4, @remear and @koryteg to be potential reviewers.

Copy link
Member

@bf4 bf4 left a comment

Choose a reason for hiding this comment

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

👍 Thanks for this! some small changes.


```ruby
@post = Post.first
render json: @post, serializer: SpecialPostSerializer
Copy link
Member

Choose a reason for hiding this comment

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

Please amend to show that you explicit serializer for a single resource can be specified by the serializer option, and for a collection of all the same resource by each_serializer. (Unfortunately,) when rending a collection, like an index action, serializer refers to the collection serializer.

@@ -382,11 +382,26 @@ The serialized value for a given key. e.g. `read_attribute_for_serialization(:ti

#### #links
Copy link
Member

Choose a reason for hiding this comment

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

extra # in front of links

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you man that #### #links should be #### links? Many h4 headers in this document have a # as part of the text. I believe this is to indicate that they are instance methods. Would you like me to remove all of them? e.g. ln 238, ln 252

Copy link
Member

Choose a reason for hiding this comment

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

Oh you're right about instance methods. Code blindness...

@cassidycodes
Copy link
Contributor Author

@bf4 Are these failing checks normal?

@bf4
Copy link
Member

bf4 commented Apr 13, 2017

@cassidycodes code, no, the failures are new.. not sure where

mustermann-1.0.0 requires ruby version >= 2.2.0, which is incompatible with the

is coming from, but I'm guessing it's from grape and I'm thinking active_model_serializers-grape should be in its own repo?

@cassidycodes
Copy link
Contributor Author

@bf4 I've squashed my commits and pushed again. Maybe those failures will mysteriously disappear too? If not I can try look into it.

- Updating general/serializers.md
- Updating docs/general/rendering.md
- adding to changelog
- Updating rendering.md to indicate that `each_serializer` must be used on a collection
- updating my handle in previous changelog entry
@cassidycodes
Copy link
Contributor Author

@bf4 This branch is up-to-date now and should be ready for a merge.

@@ -79,7 +80,7 @@ Misc:

- [#1878](https://github.com/rails-api/active_model_serializers/pull/1878) Cache key generation for serializers now uses `ActiveSupport::Cache.expand_cache_key` instead of `Array#join` by default and is also overridable. This change should be backward-compatible. (@markiz)

- [#1799](https://github.com/rails-api/active_model_serializers/pull/1799) Add documentation for setting the adapter. (@ScottKbka)
- [#1799](https://github.com/rails-api/active_model_serializers/pull/1799) Add documentation for setting the adapter. (@cassidycodes)
Copy link
Member

Choose a reason for hiding this comment

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

did you change your username? strange that this would be wrong.

@bf4 bf4 merged commit af5e9d6 into rails-api:master May 1, 2017
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.

None yet

3 participants