Follow up to #1535 #1543

groyoh · 2016-02-28T12:45:25Z

UPDATE: Merged via e30b2a4

Purpose

Follow up to #1535.
Closes #1529.

Changes

One test was renamed because it contained __
The ActiveModelSerializers::Deserialization module now call Adater instead of ActiveModelSerializers::Adapter.
The changelog was added for Moved the adapter and adapter folder to active_model_serializers folder and changed the module namespace #1535

bf4 · 2016-02-28T15:20:11Z

CHANGELOG.md

@@ -1,6 +1,8 @@
 ## 0.10.x

 Breaking changes:
+- [#1535](https://github.com/rails-api/active_model_serializers/pull/1535) Move the adapter and adapter folder to


So, I think we do want to re-add the deprecated Adapter classes...

I mean, like ArraySerializer -> CollectionSerializer per the RFC https://github.com/rails-api/active_model_serializers/blob/master/docs/rfcs/0000-namespace.md#keeping-compatibility

Yeah I realized that afterward. I'm currently working on this ;)

Should the "ApiObjects" also be re-add or should I simply change the references to match with the ActiveModelSerializers namespace?

groyoh · 2016-02-28T16:50:27Z

lib/active_model/serializer/adapter.rb

+        end
+
+        def create(resource, options = {})
+          warn "Calling deprecated ActiveModel::Serializer::Adater in #{caller[0..2].join(', ')}. Please use ActiveModelSerializers::Adapter"


Added deprecation message here.

bf4 · 2016-02-29T03:16:02Z

Hey @groyoh good stuff in here, but I think you overdid it a bit. The only interface people should be using is Adapter, Adapter::Base/Null/Json/JsonApi and those can be referenced I think as

# lib/active_model/serializer/adapter.rb
ActiveModel::Serializer::Adapter = ActiveModelSerializers::Adapter
# lib/active_model/serializer/adapter/json_api.rb
ActiveModel::Serializer::Adapter::JsonApi = ActiveModelSerializers::Adapter::JsonApi

or something like that (would need some requires.).. I think that's a good starting point. Alternative, we can use the DelegateClass or SimpleDelegator object in order to allow us to add deprecation warnings without subclassing the AMS.

groyoh · 2016-02-29T04:35:34Z

test/adapter/json_api/resource_identifier_test.rb

@@ -0,0 +1,85 @@
+require 'test_helper'


This file was simply moved. Nothing to see here.

bf4 · 2016-02-29T04:42:20Z

lib/active_model/serializer/adapter/json_api.rb

+    module Adapter
+      class JsonApi < DelegateClass(ActiveModelSerializers::Adapter::JsonApi)
+        def initialize(serializer, options = {})
+          warn "Calling deprecated ActiveModel::Serializer::Adater::JsonApi in #{caller[0..2].join(', ')}. Please use ActiveModelSerializers::Adapter::JsonApi"


typo: s/Adater/Adapter

Does this work? Calling deprecated #{self.class.name} (#{__FILE__}) from #{caller[0..2].join(', '). Please use #{self.class.name.sub('ActiveModel::Serializer', 'ActiveModelSerializers')} (not tested in terminal)

👍 it works 😄

bf4 · 2016-02-29T04:47:36Z

lib/active_model/serializer/adapter.rb

+        #     AMS::Adapter.register(:my_adapter, MyAdapter)
+        # @note The registered name strips out 'ActiveModel::Serializer::Adapter::'
+        #   so that registering 'ActiveModel::Serializer::Adapter::Json' and
+        #   'Json' will both register as 'json'.


no need for all the comments here. Can just be {ActiveModelSerializers::Adapter.register} I think

or @see ActiveModelSerializers::Adapter.register

I'll add a @deprecated tag instead.

- The removed classes and modules were added back with deprecation warning and deprecation test were added for them. - One test was renamed because it contained `__`. - Some tests were refactored. - The ActiveModelSerializers::Deserialization module is now called Adapter instead of ActiveModelSerializers::Adapter. - The changelog was added for rails-api#1535

groyoh · 2016-02-29T06:02:52Z

@bf4 I took care of all your comments. I expect it to be good to merge 😄

groyoh · 2016-02-29T06:04:28Z

test/adapter/deprecation_test.rb

+          @serializer = DeprecatedPostSerializer.new(post)
+        end
+
+        def test_null_adapter_serialization


I added these tests to have some kind of regression tests. It's not completely reliable but it's better than nothing.

Needs followup - way we deprecate classes to be generalized - initialize old json api #1543 (comment) - consider restoring ensure #1543 (comment)

bf4 · 2016-03-07T06:08:42Z

Merged by e30b2a4

bf4 · 2016-03-07T06:08:50Z

Closed via e30b2a4

I accidentally introduced the duplication when merging rails-api#1543 (comment) and they've evolved separately since then. They both have some value and need to be combined.

* Really fix intermittent relationship test failures * Unify the two JSON API relationship test files I accidentally introduced the duplication when merging #1543 (comment) and they've evolved separately since then. They both have some value and need to be combined. * Resolve duplicate tests from diverged tests files * No longer test Association/Relationship interface directly

groyoh force-pushed the follow_up_1535 branch from c54adab to b0fdb0d Compare February 28, 2016 12:49

groyoh added the C: Cleanup label Feb 28, 2016

bf4 reviewed Feb 28, 2016
View reviewed changes

groyoh force-pushed the follow_up_1535 branch from b0fdb0d to e6fdeac Compare February 28, 2016 16:45

groyoh reviewed Feb 28, 2016
View reviewed changes

groyoh force-pushed the follow_up_1535 branch from e6fdeac to d06b141 Compare February 28, 2016 16:59

groyoh added the S: Needs Code Review label Feb 28, 2016

groyoh mentioned this pull request Feb 28, 2016

Moved the adapter and adapter folder to active_model_serializers folder and changed the module namespace #1535

Merged

7 tasks

groyoh force-pushed the follow_up_1535 branch from d06b141 to 1af55a2 Compare February 29, 2016 04:30

groyoh reviewed Feb 29, 2016
View reviewed changes

groyoh force-pushed the follow_up_1535 branch from 1af55a2 to d935acc Compare February 29, 2016 04:39

bf4 reviewed Feb 29, 2016
View reviewed changes

groyoh force-pushed the follow_up_1535 branch from d935acc to 505c637 Compare February 29, 2016 04:44

bf4 reviewed Feb 29, 2016
View reviewed changes

groyoh force-pushed the follow_up_1535 branch 3 times, most recently from 0f789e4 to 8e0b877 Compare February 29, 2016 05:48

groyoh force-pushed the follow_up_1535 branch from 8e0b877 to 6829069 Compare February 29, 2016 06:01

groyoh reviewed Feb 29, 2016
View reviewed changes

groyoh mentioned this pull request Mar 3, 2016

Move ApiObjects to JsonApi namespace/folder #1529

Closed

bf4 added a commit that referenced this pull request Mar 7, 2016

Merge branch 'groyoh-follow_up_1535'

e30b2a4

Needs followup - way we deprecate classes to be generalized - initialize old json api #1543 (comment) - consider restoring ensure #1543 (comment)

bf4 closed this Mar 7, 2016

bf4 mentioned this pull request Mar 7, 2016

Add a deprecation DSL #1558

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Follow up to #1535 #1543

Follow up to #1535 #1543

groyoh commented Feb 28, 2016

bf4 Feb 28, 2016

bf4 Feb 28, 2016

groyoh Feb 28, 2016

groyoh Feb 28, 2016

groyoh Feb 28, 2016

bf4 commented Feb 29, 2016

groyoh Feb 29, 2016

bf4 Feb 29, 2016

groyoh Feb 29, 2016

bf4 Feb 29, 2016

bf4 Feb 29, 2016

groyoh Feb 29, 2016

groyoh commented Feb 29, 2016

groyoh Feb 29, 2016

bf4 commented Mar 7, 2016

bf4 commented Mar 7, 2016

Follow up to #1535 #1543

Follow up to #1535 #1543

Conversation

groyoh commented Feb 28, 2016

Purpose

Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bf4 commented Feb 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

groyoh commented Feb 29, 2016

Choose a reason for hiding this comment

bf4 commented Mar 7, 2016

bf4 commented Mar 7, 2016