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

Add documentation for root #1959

Merged

Conversation

shunsuke227ono
Copy link
Contributor

Purpose

Provide a document for root at serializers#root.

Changes

Adds new lines to docs/general/serializers.md.

Caveats

Nothing.

Related GitHub issues

No issue.

Additional helpful information

@mention-bot
Copy link

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

@shunsuke227ono shunsuke227ono changed the title add document for root Add documentation for root Oct 26, 2016
Copy link
Member

@groyoh groyoh left a comment

Choose a reason for hiding this comment

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

Awesome job 💯 . There are some small changes to be made before merging but it really good. Could you please add an entry to the CHANGELOG?

* [Setting `type`](https://github.com/rails-api/active_model_serializers/blob/master/docs/general/serializers.md#type)
* Passing an argument like `root: 'specific_name'` to the serializer's initialization.

You can also see examples of root at linked pages above.
Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by that sentence exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This means you can find some examples how 'root' is used in the pages linked above. I added this line because I thought it might be easier for reader if they can find some examples of usage. But now I guess maybe this is not really needed. So I will remove this line!

There are some ways to specify root like below:
* [Overriding the root key](https://github.com/rails-api/active_model_serializers/blob/master/docs/general/rendering.md#overriding-the-root-key)
* [Setting `type`](https://github.com/rails-api/active_model_serializers/blob/master/docs/general/serializers.md#type)
* Passing an argument like `root: 'specific_name'` to the serializer's initialization.
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather change it a bit to something like:

-* Passing an argument like `root: 'specific_name'` to the serializer's initialization.
+* Specifying the `root` option, e.g. `root: 'specific_name'`, during the serializer's initialization:
+
+```ruby
+ActiveModelSerializers::SerializableResource.new(foo, root: 'bar')
+```

@@ -217,7 +217,15 @@ The object being serialized.

#### #root

PR please :)
Resource root which is included in `JSON` adapter. As you can see at [Adapters Document](https://github.com/rails-api/active_model_serializers/blob/master/docs/general/adapters.md), `Attribute` adapter (default) and `JSON API` adapter dose not include root at top level.
By default, the resource root comes from the class name of the serialized class.
Copy link
Member

Choose a reason for hiding this comment

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

There is a small mistake here. Could you change it to:

-By default, the resource root comes from the class name of the serialized class.
+By default, the resource root comes from the `model_name` of the serialized object's class.

Copy link
Member

Choose a reason for hiding this comment

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

should be a relative link.

s/dose/does

Resource root which is included in `JSON` adapter. As you can see at [Adapters Document](https://github.com/rails-api/active_model_serializers/blob/master/docs/general/adapters.md), `Attribute` adapter (default) and `JSON API` adapter dose not include root at top level.
By default, the resource root comes from the class name of the serialized class.

There are some ways to specify root like below:
Copy link
Member

Choose a reason for hiding this comment

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

s/some ways/several

✂️ like below

@@ -217,7 +217,15 @@ The object being serialized.

#### #root
Copy link
Member

Choose a reason for hiding this comment

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

lazy ref to discussion of some of the various way root can be configured and what we want to do about it https://github.com/rails-api/active_model_serializers/pull/1794/files

@shunsuke227ono
Copy link
Contributor Author

@groyoh @bf4 Thanks for reviewing! I have updated based on the reviews. Could you check and merge if it's okay?

* Specifying the `root` option, e.g. `root: 'specific_name'`, during the serializer's initialization:

```ruby
+ActiveModelSerializers::SerializableResource.new(foo, root: 'bar')
Copy link
Member

Choose a reason for hiding this comment

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

There is a + that sneaked into the ruby code here. Once you remove it I'll merge 😉 Thanks for the work!

@shunsuke227ono
Copy link
Contributor Author

shunsuke227ono commented Nov 1, 2016

@groyoh oops! I have removed the + from the line! Please merge it :)

@groyoh groyoh merged commit ce8dd36 into rails-api:master Nov 1, 2016
GregPK pushed a commit to GregPK/active_model_serializers that referenced this pull request Apr 25, 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

4 participants