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.

Sign up for GitHub

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

Jump to bottom

Move SerializableResource to ActiveModelSerializers namespace #1608

Merged
merged 1 commit into from
Mar 30, 2016
Merged

Move SerializableResource to ActiveModelSerializers namespace #1608

merged 1 commit into from
Mar 30, 2016

Conversation

groyoh
Copy link
Member

@groyoh groyoh commented Mar 19, 2016

Purpose

Moves SerializableResource to ActiveModelSerializers namespace based on https://github.com/groyoh/active_model_serializers/blob/master/docs/rfcs/0000-namespace.md

Changes

  • ActiveModel::SerializableResource code was moved to ActiveModelSerializers::SerializableResource
  • ActiveModel::SerializableResource is flagged as deprecated.
  • All the references were updated

Related GitHub issues

#1310

@groyoh groyoh force-pushed the move_serializable_resource branch from 0ee322d to ee37501 Compare March 19, 2016 06:39
groyoh
groyoh reviewed Mar 19, 2016
View reviewed changes
lib/active_model/serializer/reflection.rb
@@ -38,6 +38,7 @@ def initialize(*)
super
@_links = {}
@_include_data = true
@_meta = nil
Copy link
Member Author

Choose a reason for hiding this comment

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

Fix related to ruby warnings. Not sure how these fixes got into this commit 😕

@bf4
Copy link
Member

bf4 commented Mar 21, 2016

👍 Some questions about the Ruby warnings, and any discussion of naming while we're here.

@bf4 bf4 added the C: Cleanup label Mar 21, 2016
@groyoh
Copy link
Member Author

groyoh commented Mar 21, 2016

Should we actually care about these warnings? I think I should get rid of the warnings changes from this commit as it has nothing to do with the commit.

groyoh
groyoh reviewed Mar 21, 2016
View reviewed changes
CHANGELOG.md
@@ -35,6 +35,7 @@ Fixes:
- [#1488](https://github.com/rails-api/active_model_serializers/pull/1488) Require ActiveSupport's string inflections (@nate00)

Misc:
- [#1608](https://github.com/rails-api/active_model_serializers/pull/1608) Move SerializaResource to ActiveModelSerializers (@groyoh)
Copy link
Member Author

Choose a reason for hiding this comment

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

typo, need to fix it

@bf4
Copy link
Member

bf4 commented Mar 21, 2016

@groyoh If your PR introduced Ruby warnings, then yes, address them. If they're pre-existing, and relevant to what you're doing, then probably. Otherwise, you can leave them.

@groyoh groyoh force-pushed the move_serializable_resource branch 2 times, most recently from cde9874 to b72fb05 Compare March 27, 2016 08:39
@groyoh
Copy link
Member Author

groyoh commented Mar 27, 2016

Rebased and removed the warning changes that were not related to my PR.

@groyoh groyoh force-pushed the move_serializable_resource branch 2 times, most recently from a8e12f6 to ab49a40 Compare March 30, 2016 09:28
@groyoh groyoh force-pushed the move_serializable_resource branch from ab49a40 to 21cb896 Compare March 30, 2016 09:33
@groyoh
Copy link
Member Author

groyoh commented Mar 30, 2016

Rebased again. @bf4 @NullVoxPopuli anyway you could review this and merge it before merging any other PR? Since there are a lot of files involved, there are merge conflicts quite often.

@NullVoxPopuli
Copy link
Contributor

👍 Good work!

@NullVoxPopuli NullVoxPopuli merged commit 3963956 into rails-api:master Mar 30, 2016
NullVoxPopuli
NullVoxPopuli reviewed Mar 30, 2016
View reviewed changes
test/serializable_resource_test.rb
@serializable_resource = SerializableResource.new(@resource)
end

def test_deprecation
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@groyoh
Copy link
Member Author

groyoh commented Mar 30, 2016

@NullVoxPopuli thanks for reviewing 🎉

@groyoh groyoh deleted the move_serializable_resource branch March 30, 2016 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
C: Cleanup V: 0.10.x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@groyoh @bf4 @NullVoxPopuli