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

Correct comment #1887

Merged
merged 1 commit into from
Aug 31, 2016
Merged

Correct comment #1887

merged 1 commit into from
Aug 31, 2016

Conversation

johnnymo87
Copy link
Contributor

Purpose

Make the comment reflect what the function does

Changes

Correct comment

@mention-bot
Copy link

@johnnymo87, thanks for your PR! By analyzing the annotation information on this pull request, we identified @groyoh and @bf4 to be potential reviewers

@NullVoxPopuli
Copy link
Contributor

Nice find! Would you like to add a changelog to take credit for the correction?

@@ -49,7 +49,7 @@ def serializer_instance

# Get serializer either explicitly :serializer or implicitly from resource
# Remove :serializer key from serializer_opts
# Replace :serializer key with :each_serializer if present
# Replace :each_serializer key with :serializer if present
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I don't think it's exactly clear either way, now that I look at it. Should be

Remove :each_serializer if present and set as :serializer key

Copy link
Member

Choose a reason for hiding this comment

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

(or something like that)

@johnnymo87
Copy link
Contributor Author

@NullVoxPopuli @bf4 updated

@bf4 bf4 merged commit 0102434 into rails-api:master Aug 31, 2016
@bf4
Copy link
Member

bf4 commented Aug 31, 2016

@johnnymo87 Merged. Sorry I missed this was ready.

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

Successfully merging this pull request may close these issues.

None yet

4 participants