Update the RFC to use ActiveModelSerializers

After some internal discussion was decided to use the ActiveModelSerializers
namespace.

This patch update the content following this idea.

Ref:
https://github.com/rails-api/active_model_serializers/pull/1310/files#r45947587
https://github.com/rails-api/active_model_serializers/pull/1310/files#r47144210
This commit is contained in:
Mauro George 2016-01-06 19:55:21 -02:00
parent a183645ed4
commit 2f8c430a09

View File

@ -1,6 +1,6 @@
- Start Date: (2015-10-29) - Start Date: (2015-10-29)
- RFC PR: (leave this empty) - RFC PR: https://github.com/rails-api/active_model_serializers/pull/1310
- AMS Issue: (leave this empty) - ActiveModelSerializers Issue: https://github.com/rails-api/active_model_serializers/issues/1298
# Summary # Summary
@ -25,60 +25,32 @@ At `ActiveModel` we have:
- `ActiveModel::SerializableResource` - `ActiveModel::SerializableResource`
The idea here is to provide a single namespace `ActiveModel::Serializers` to the user. The idea here is to provide a single namespace `ActiveModelSerializers` to the user.
Following the same idea we have on other gems like Following the same idea we have on other gems like
[Devise](https://github.com/plataformatec/devise/blob/e9c82472ffe7c43a448945f77e034a0e47dde0bb/lib/devise.rb), [Devise](https://github.com/plataformatec/devise/blob/e9c82472ffe7c43a448945f77e034a0e47dde0bb/lib/devise.rb),
[Refile](https://github.com/refile/refile/blob/6b24c293d044862dafbf1bfa4606672a64903aa2/lib/refile.rb) and [Refile](https://github.com/refile/refile/blob/6b24c293d044862dafbf1bfa4606672a64903aa2/lib/refile.rb) and
[Active Job](https://github.com/rails/rails/blob/30bacc26f8f258b39e12f63fe52389a968d9c1ea/activejob/lib/active_job.rb) [Active Job](https://github.com/rails/rails/blob/30bacc26f8f258b39e12f63fe52389a968d9c1ea/activejob/lib/active_job.rb)
for example. for example.
This way we are clarifing the boundaries of
[ActiveModelSerializers and Rails](https://github.com/rails-api/active_model_serializers/blob/master/CHANGELOG.md#prehistory)
and make clear that the `ActiveModel::Serializer` class is no longer the primary
behavior of the ActiveModelSerializers.
# Detailed design # Detailed design
## Require statement and main module
We are adding a extension for the Active Model, so
[following the Rubygens recomendation](http://guides.rubygems.org/name-your-gem/)
for the gem name we need to change to this.
|Gem name | Require statement | Main class or module |
|--------------------------|----------------------------|--------------------------|
| active_model_serializers | `active_model/serializers` | ActiveModel::Serializers |
The expected gem name, in the gemspec is `active_model-serializers` but we don't
need to change this, we can change the code without the need of a new gem on Rubygems.
Active Model for example follow the same idea the gem name on gemspec is `activemodel` and to the end user is:
|Gem name | Require statement | Main class or module |
|--------------------------|----------------------------|--------------------------|
| activemodel | `active_model` | ActiveModel |
As you can see we do not require `activemodel`(the gem name in gemspec) insted
we use `active_model`.
And based on the [bump of `0.10.0.pre` released by Steve Klabnik](https://github.com/rails-api/active_model_serializers/tree/86fc7d7227f3ce538fcb28c1e8c7069ce311f0e1)
if we take a look on README and gemspec always is used `ActiveModel::Serializers`.
## New classes and modules organization ## New classes and modules organization
Since this will be a big change we can do this on baby steps, read small PRs. A Since this will be a big change we can do this on baby steps, read small pull requests. A
possible approach is: possible approach is:
- Create the `ActiveModel::Serializers` namespace; - All new code will be in `lib/active_model_serializers/` using
- Move all content under `ActiveModelSerializers` to be under the module namespace `ActiveModelSerializers`.
`ActiveModel::Serializers`, the logger is on this step;
- Move all content under `ActiveModel::Serializer` to be under - Move all content under `ActiveModel::Serializer` to be under
`ActiveModel::Serializers`, the adapter is on this steps; `ActiveModelSerializers`, the adapter is on this steps;
- Move all content under `ActiveModel` to be under `ActiveModel::Serializers`, - Move all content under `ActiveModel` to be under `ActiveModelSerializers`,
the `SerializableResource` is on this step; the `SerializableResource` is on this step;
- Now that all the code lives under `ActiveModel::Serializers` we can: - Change all public API that doesn't make sense, keeping in mind only to keep
- create a better name to the `ActiveModel::Serializers::Serializer`
keeping in mind only to keep this in the same namespace
- create a better name to the `ActiveModel::Serializers::Serializer::Adapter::JsonApi`
probably remove this from the `ActiveModel::Serializers::Serializer`
and do something like `ActiveModel::Serializers::Adapter::JsonApi`
keeping in mind only to keep this in the same namespace
- Change all public API that doesn't make sense, keeping in mind only to keep
this in the same namespace this in the same namespace
- Update the README; - Update the README;
- Update the docs; - Update the docs;
@ -87,29 +59,48 @@ The following table represents the current and the desired classes and modules
at the first moment. at the first moment.
| Current | Desired | Notes | | Current | Desired | Notes |
|-------------------------------------------------------|--------------------------------------------------|--------------------| |--------------------------------------------------------|--------------------------------------------------|--------------------|
|`ActiveModelSerializers` and `ActiveModel::Serializer` | `ActiveModel::Serializers` | The main namespace | | `ActiveModelSerializers` and `ActiveModel::Serializer` | `ActiveModelSerializers` | The main namespace |
| `ActiveModelSerializers.logger` | `ActiveModel::Serializers.logger` || | `ActiveModelSerializers.logger` | `ActiveModelSerializers.logger` ||
|`ActiveModelSerializers::Model` | `ActiveModel::Serializers::Model` || | `ActiveModelSerializers::Model` | `ActiveModelSerializers::Model` ||
|`ActiveModel::SerializableResource` | `ActiveModel::Serializers::SerializableResource` || | `ActiveModel::SerializableResource` | `ActiveModelSerializers::SerializableResource` ||
| `ActiveModel::Serializer` | `ActiveModel::Serializers::Serializer` | I know that is probably a bad name, but In a second moment we can rename this to `Resource` [for example following this idea](https://github.com/rails-api/active_model_serializers/pull/1301/files#r42963185)| | `ActiveModel::Serializer` | `ActiveModelSerializers::Serializer` | The name can be discussed in a future pull request. For example, we can rename this to `Resource` [following this idea](https://github.com/rails-api/active_model_serializers/pull/1301/files#r42963185) more info about naming in the next section|
|`ActiveModel::Serializer.config` | `ActiveModel::Serializers.config` || | `ActiveModel::Serializer.config` | `ActiveModelSerializers.config` ||
## 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`.
Discussion of renaming existing classes / modules and JsonApi objects will
happen in separate pull requests, and issues, and in the google doc
https://docs.google.com/document/d/1rcrJr0sVcazY2Opd_6Kmv1iIwuHbI84s1P_NzFn-05c/edit?usp=sharing
Some of names already have a definition.
- Adapters get their own namespace under ActiveModelSerializers. E.g
`ActiveModelSerializers::Adapter`
- Serializers get their own namespace under ActiveModelSerializers. E.g
`ActiveModelSerializers::Serializer`
## Keeping compatibility
All moved classes or modules be aliased to their old name and location with
deprecation warnings, such as
[was done for CollectionSerializer](https://github.com/rails-api/active_model_serializers/pull/1251).
# Drawbacks # Drawbacks
This will be a breaking change, so all users serializers will be broken. This will be a breaking change, so all users serializers will be broken after a
All PRs will need to rebase since the architeture will change a lot. major bump.
All pull requests will need to rebase since the architeture will change a lot.
# Alternatives # Alternatives
We can keep the way it is, and keep in mind to not add another namespace as a We can keep the way it is, and keep in mind to not add another namespace as a
public API. public API.
Or we can start moving the small ones that seems to be the
`ActiveModelSerializers` and `ActiveModel` and later we can handle the
`ActiveModel::Serializer`.
# Unresolved questions # Unresolved questions
What is the better class name to be used to the class that will be inherited at What is the better class name to be used to the class that will be inherited at
the creation of a serializer. the creation of a serializer. This can be discussed in other RFC or directly via
pull request.