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

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

Conversation

bf4
Copy link
Member

@bf4 bf4 commented Feb 24, 2016

This is a squashed commit of #1499 with master then merged in (rebasing got complicated).

Closes #1499 #1446

Work is per https://github.com/rails-api/active_model_serializers/blob/master/docs/rfcs/0000-namespace.md

Original Description

Changed the namespace in adapters and folder to active_model_serializers from active_model::serializer

Changed namespace of adapters in serializers and other folders

Moved adapter_for_test file to active_model_serializers folder and changed namespace of adapter inside the test file

Require ActiveSupport's string/inflections

We depend on string/inflections to define String#underscore.

Refactor JsonApi adapter to avoid redundant computations.

Update readme.md to link to v0.10.0.rc4

changed namespace of adapter folder testcases

Changed all namespaces of adapter under active_moder_serializers

Namespaced IncludeTree which is from serializer module, so needed to namespace it properly

Fixed wrong namsepacing of fieldset

namespace change in deserializer json_api

Fixed the namespace for collection serializer when used inside adapter, changed namespace for adapter to new namespace which I had forgotten previously

Modified logging test and adapter test cases to make the testcases pass

Changed the yardoc links,as old links are not taking to documentation pages,proper links for 0.10,0.9 and 0.8 in rubydoc

Rubocop errors are fixed by underscore naming unused variables

Moved the require of adapter to serializable resource

Remoeved adapter dependency inside serializer and added warning to Serializer::adapter method

Fixed frament cache test which is calling Serializer.adapter

Changed the name of lookup_adapter_from_config to configured_adapter

Changed the docs which will show the new namespace of adapters

Rubocop fix

domitian and others added 2 commits February 23, 2016 21:49
…er and changed the module namespace

Changed the namespace in adapters and folder to active_model_serializers from active_model::serializer

Changed namespace of adapters in serializers and other folders

Moved adapter_for_test file to active_model_serializers folder and changed namespace of adapter inside the test file

Require ActiveSupport's string/inflections

We depend on string/inflections to define String#underscore.

Refactor JsonApi adapter to avoid redundant computations.

Update readme.md to link to v0.10.0.rc4

changed namespace of adapter folder testcases

Changed all namespaces of adapter under active_moder_serializers

Namespaced IncludeTree which is from serializer module, so needed to namespace it properly

Fixed wrong namsepacing of fieldset

namespace change in deserializer json_api

Fixed the namespace for collection serializer when used inside adapter, changed namespace for adapter to new namespace which I had forgotten previously

Modified logging test and adapter test cases to make the testcases pass

Changed the yardoc links,as old links are not taking to documentation pages,proper links for 0.10,0.9 and 0.8 in rubydoc

Rubocop errors are fixed by underscore naming unused variables

Moved the require of adapter to serializable resource

Remoeved adapter dependency inside serializer and added warning to Serializer::adapter method

Fixed frament cache test which is calling Serializer.adapter

Changed the name of lookup_adapter_from_config to configured_adapter

Changed the docs which will show the new namespace of adapters

Rubocop fix
…ve-model-serializers

Conflicts:
	CHANGELOG.md
	lib/active_model/serializer/adapter/attributes.rb
	lib/active_model/serializer/adapter/cached_serializer.rb
	lib/active_model/serializer/adapter/fragment_cache.rb
	lib/active_model/serializer/adapter/json_api.rb
	lib/active_model/serializer/adapter/json_api/link.rb
	test/adapter/fragment_cache_test.rb
	test/adapter/json_api/links_test.rb
	test/adapter/json_api/resource_type_config_test.rb
@@ -70,7 +70,7 @@ at the first moment.
## Renaming of class and modules

When moving some content to the new namespace we can find some names that does
not make much sense like `ActiveModelSerializers::Serializer::Adapter::JsonApi`.
not make much sense like `ActiveModelSerializers::Adapter::JsonApi`.
Copy link
Member

Choose a reason for hiding this comment

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

Should this actually change?

Copy link
Member Author

Choose a reason for hiding this comment

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

😊 oops

@groyoh
Copy link
Member

groyoh commented Feb 24, 2016

Looks good to me 👍
@domitian and @bf4 Amazing work here.

@bf4
Copy link
Member Author

bf4 commented Feb 25, 2016

@groyoh Thanks for the great review. I ended up doing some brakeman work on rubygems and didn't get to this tonight :(

@bf4
Copy link
Member Author

bf4 commented Feb 26, 2016

@groyoh @domitian Responded to feedback and updated. The PR is so big I expect there to be some bugs from it, but they should be pretty easy to fix.

@bf4
Copy link
Member Author

bf4 commented Feb 26, 2016

We should keep the old adapters in place with warnings, I think.

just

  • lib/active_model/serializer/adapter.rb
  • lib/active_model/serializer/adapter/base.rb
  • lib/active_model/serializer/adapter/json.rb
  • lib/active_model/serializer/adapter/json_api.rb
  • lib/active_model/serializer/adapter/attributes.rb
before:
lib/active_model/serializer
├── adapter
│   ├── attributes.rb
│   ├── base.rb
│   ├── cached_serializer.rb
│   ├── fragment_cache.rb
│   ├── json
│   │   └── fragment_cache.rb
│   ├── json.rb
│   ├── json_api
│   │   ├── api_objects
│   │   │   ├── relationship.rb
│   │   │   └── resource_identifier.rb
│   │   ├── api_objects.rb
│   │   ├── deserialization.rb
│   │   ├── fragment_cache.rb
│   │   ├── link.rb
│   │   ├── meta.rb
│   │   └── pagination_links.rb
│   ├── json_api.rb
│   └── null.rb
├── adapter.rb
├── array_serializer.rb
├── association.rb
├── associations.rb
├── attribute.rb
├── attributes.rb
├── belongs_to_reflection.rb
├── caching.rb
├── collection_reflection.rb
├── collection_serializer.rb
├── configuration.rb
├── field.rb
├── fieldset.rb
├── has_many_reflection.rb
├── has_one_reflection.rb
├── include_tree.rb
├── links.rb
├── lint.rb
├── meta.rb
├── reflection.rb
├── singular_reflection.rb
├── type.rb
└── version.rb
after:
lib/active_model/serializer
├── adapter
│   └── json_api
│       ├── api_objects
│       │   ├── relationship.rb
│       │   └── resource_identifier.rb
│       ├── api_objects.rb
│       └── meta.rb
├── array_serializer.rb
├── association.rb
├── associations.rb
├── attribute.rb
├── attributes.rb
├── belongs_to_reflection.rb
├── caching.rb
├── collection_reflection.rb
├── collection_serializer.rb
├── configuration.rb
├── field.rb
├── fieldset.rb
├── has_many_reflection.rb
├── has_one_reflection.rb
├── include_tree.rb
├── links.rb
├── lint.rb
├── meta.rb
├── reflection.rb
├── singular_reflection.rb
├── type.rb
└── version.rb
lib/active_model_serializers
├── adapter
│   └── json_api
│       └── api_objects
├── caching
├── callbacks.rb
├── deserialization.rb
├── logging.rb
├── model.rb
├── railtie.rb
├── serialization_context.rb
├── test
│   ├── schema.rb
│   └── serializer.rb
└── test.rb
lib/active_model_serializers
├── adapter
│   ├── attributes.rb
│   ├── base.rb
│   ├── cached_serializer.rb
│   ├── fragment_cache.rb
│   ├── json
│   │   └── fragment_cache.rb
│   ├── json.rb
│   ├── json_api
│   │   ├── api_objects
│   │   ├── deserialization.rb
│   │   ├── fragment_cache.rb
│   │   ├── link.rb
│   │   └── pagination_links.rb
│   ├── json_api.rb
│   └── null.rb
├── adapter.rb
├── caching
├── callbacks.rb
├── deserialization.rb
├── logging.rb
├── model.rb
├── railtie.rb
├── serialization_context.rb
├── test
│   ├── schema.rb
│   └── serializer.rb
└── test.rb

end

def process_resource(serializer, primary)
resource_identifier = ActiveModel::Serializer::Adapter::JsonApi::ApiObjects::ResourceIdentifier.new(serializer).as_json
Copy link
Member

Choose a reason for hiding this comment

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

@bf4 I just realized that the two classes under ApiObjects were not moved. What's the thought behind this? Waiting for #1529 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, just is more scope for me :) ref #1535 (comment) I'd rather put my energies elsewhere. I was really just rebasing the original pr

@groyoh
Copy link
Member

groyoh commented Feb 28, 2016

@bf4 sorry for the delay, I've been pretty busy the last few days. I'm merging this and I'll work on follow up PRs.

groyoh pushed a commit that referenced this pull request Feb 28, 2016
…to-active-model-serializers

Moved the adapter and adapter folder to active_model_serializers folder and changed the module namespace
@groyoh groyoh merged commit 8a04005 into rails-api:master Feb 28, 2016
groyoh pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 28, 2016
- One test was renamed because it contained `__`
- The ActiveModelSerializers::Deserialization module now call Adater
  instead of ActiveModelSerializers::Adapter.
- The changelog was added for rails-api#1535
@groyoh groyoh mentioned this pull request Feb 28, 2016
groyoh pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 28, 2016
- One test was renamed because it contained `__`
- The ActiveModelSerializers::Deserialization module now call Adater
  instead of ActiveModelSerializers::Adapter.
- The changelog was added for rails-api#1535
@bf4 bf4 deleted the domitian-move-namespace-of-adapter-to-active-model-serializers branch February 28, 2016 15:19
groyoh pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 28, 2016
- 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 pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 28, 2016
- 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
@frank06
Copy link

frank06 commented Feb 28, 2016

Guys just a heads up! I have no idea what this PR is about.

I have AMS master branch in a project Gemfile, last night it worked fine, this morning it started throwing ArgumentError (wrong number of arguments (2 for 0)): upon serialization.

BTW: Any idea when will 0.10 see the light? Or even a pre-release? It's been almost 6 months.

@groyoh
Copy link
Member

groyoh commented Feb 28, 2016

The goal of this PR was to move ActiveModel::Serializer::Adapter namespace to ActiveModelSerializers::Adapter (ref. https://github.com/rails-api/active_model_serializers/blob/master/docs/rfcs/0000-namespace.md).
The PR removed the existing adapter, but they will be brought back with deprecation warning in #1543. This is probably why it's not working.

Could you give more details about your error e.g. stacktrace, code, etc. and create an issue for it?

Also please stick to 0.10.0.rc4 or a specific commit ref as I assume master may have some others breaking changes in the future.

@bf4
Copy link
Member Author

bf4 commented Feb 28, 2016

Frank, sorry re broken master. I think it should have to do with the
changing the adapter class names, and will be fixed soonish in @groyoh's
pr. Releases are another story. Let's discuss in the slack,
amserializers.herokuapp.com
On Sun, Feb 28, 2016 at 3:28 PM Frank Treacy [email protected]
wrote:

Guys just a heads up! I have no idea what this PR is about.

I have AMS master branch in a project Gemfile, last night it worked fine,
this morning it started throwing ArgumentError (wrong number of arguments
(2 for 0)): upon serialization.

BTW: Any idea when will 0.10 see the light? Or even a pre-release? It's
been almost 6 months.


Reply to this email directly or view it on GitHub
#1535 (comment)
.

@bf4
Copy link
Member Author

bf4 commented Feb 29, 2016

Generally we've been advising to stay on master because it's usually stable and our release process is kind of strange right now. We're tagging this rc4 but it should probsbly be beta4 or something like that. I'd love to call it 0.10 and then release updates per semver, but we'd need to give up our defn of what's in '0.10'. Cc @groyoh

B mobile phone

On Feb 28, 2016, at 3:54 PM, Yohan Robert [email protected] wrote:

The goal of this PR was to move ActiveModel::Serializer::Adapter namespace to ActiveModelSerializers::Adapter (ref. https://github.com/rails-api/active_model_serializers/blob/master/docs/rfcs/0000-namespace.md).
The PR removed the existing adapter, but they will be brought back with deprecation warning in #1543. This is probably why it's not working.

Could you give more details about your error e.g. stacktrace, code, etc. and create an issue for it?

Also please stick to 0.10.0.rc4 or a specific commit ref as I assume master may have some others breaking changes in the future.


Reply to this email directly or view it on GitHub.

groyoh pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 29, 2016
- 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 pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 29, 2016
- 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 pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 29, 2016
- 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 pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 29, 2016
- 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 pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 29, 2016
- 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 pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 29, 2016
- 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 pushed a commit to groyoh/active_model_serializers that referenced this pull request Feb 29, 2016
- 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
@frank06
Copy link

frank06 commented Feb 29, 2016

Thanks! and no worries about the broken master. I wasn't aware of 0.10.0.rc4 (I was checking the releases tab but I missed the tags). Great job guys, I look forward to the next solid release

bf4 pushed a commit that referenced this pull request Mar 7, 2016
- 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 #1535
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.

4 participants