Move namespace of adapter to active model serializers #1499
Conversation
…er and changed the module namespace
…ers from active_model::serializer
…anged namespace of adapter inside the test file
…namespace it properly
…r, changed namespace for adapter to new namespace which I had forgotten previously
…s into move-namespace-of-adapter-to-active-model-serializers
|
Should I make code changes to make it pass for rubocop as well? |
|
Yup :) Thanks! B mobile phone
|
|
@bf4 all ci checks are fine. |
| @@ -3,6 +3,7 @@ module ActiveModel | |||
| class SerializableResource | |||
| ADAPTER_OPTION_KEYS = Set.new([:include, :fields, :adapter, :meta, :meta_key, :links]) | |||
| include ActiveModelSerializers::Logging | |||
| require 'active_model_serializers/adapter' | |||
There was a problem hiding this comment.
any require you put the require here and not at the top?
|
Looks good. Your thoughts re: #1499 (comment) |
| @@ -21,7 +21,7 @@ class Serializer | |||
| include Caching | |||
| include Links | |||
| include Type | |||
| require 'active_model/serializer/adapter' | |||
| require 'active_model_serializers/adapter' | |||
There was a problem hiding this comment.
I think this should be removed now, along with the adapter method (which I guess we can keep around with a deprecation warnings)
There was a problem hiding this comment.
I removed the adapter dependency and added adapter method with deprecation warning. To replace adapter method, I created a similar one in Adapter module called lookup_from_config. Can you look at the changes I made in the separate branch I made domitian/active_model_serializers@move-namespace-of-adapter-to-active-model-serializers...domitian:removing_adapter_from_serializer#diff-5849afc4f57a4ff11910f672112e293aR16
There was a problem hiding this comment.
I think it's good. It has some extra spacing in one of the methods.
Maybe call it configured_adapter?
There was a problem hiding this comment.
that seems to be a good name.
…rializer::adapter method
|
@bf4 I believe I made all the changes that are necessary for this PR, do you think are there any changes that needs to be done before I merge latest master into this branch(Seems to be lot of changes yesterday)? |
|
|
||
| There are two ways to register an adapter: | ||
|
|
||
| 1) The simplest, is to subclass `ActiveModel::Serializer::Adapter::Base`, e.g. the below will | ||
| 1) The simplest, is to subclass `ActiveModelSerializers::Adapter::Base`, e.g. the below will | ||
| register the `Example::UsefulAdapter` as `"example/useful_adapter"`. |
There was a problem hiding this comment.
Did you want example/useful_adapter quoted in the inline code block, or is just the code block sufficient in this context? "example/useful_adapter" vs example/useful_adapter
There was a problem hiding this comment.
It's "example/useful_adapter", though it can be looked up as `:"example/useful_adapter" if you want
# code that registers the adapter `name` to the `klass`
name = name.to_s.gsub(/\AActiveModel::Serializer::Adapter::/, ''.freeze)
adapter_map.update(name.underscore => klass)so, I think that's correct
|
Added a few comments. This looks good to me. 👍 |
#1446 will be fixed. Adapter moved from serializers to active_model_serializers namespace.
@bf4 need your help rebasing commits and coderefactoring.