From 2f8c430a093e2fe56b6e4a271eccf8d811d24e29 Mon Sep 17 00:00:00 2001 From: Mauro George Date: Wed, 6 Jan 2016 19:55:21 -0200 Subject: [PATCH] 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 --- docs/rfcs/0000-namespace.md | 107 +++++++++++++++++------------------- 1 file changed, 49 insertions(+), 58 deletions(-) diff --git a/docs/rfcs/0000-namespace.md b/docs/rfcs/0000-namespace.md index 4a5364b7..9cff50a3 100644 --- a/docs/rfcs/0000-namespace.md +++ b/docs/rfcs/0000-namespace.md @@ -1,6 +1,6 @@ - Start Date: (2015-10-29) -- RFC PR: (leave this empty) -- AMS Issue: (leave this empty) +- RFC PR: https://github.com/rails-api/active_model_serializers/pull/1310 +- ActiveModelSerializers Issue: https://github.com/rails-api/active_model_serializers/issues/1298 # Summary @@ -25,91 +25,82 @@ At `ActiveModel` we have: - `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 [Devise](https://github.com/plataformatec/devise/blob/e9c82472ffe7c43a448945f77e034a0e47dde0bb/lib/devise.rb), [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) 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 -## 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 -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: -- Create the `ActiveModel::Serializers` namespace; -- Move all content under `ActiveModelSerializers` to be under - `ActiveModel::Serializers`, the logger is on this step; +- All new code will be in `lib/active_model_serializers/` using + the module namespace `ActiveModelSerializers`. - Move all content under `ActiveModel::Serializer` to be under - `ActiveModel::Serializers`, the adapter is on this steps; -- Move all content under `ActiveModel` to be under `ActiveModel::Serializers`, + `ActiveModelSerializers`, the adapter is on this steps; +- Move all content under `ActiveModel` to be under `ActiveModelSerializers`, the `SerializableResource` is on this step; -- Now that all the code lives under `ActiveModel::Serializers` we can: - - 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 +- Change all public API that doesn't make sense, keeping in mind only to keep + this in the same namespace - Update the README; - Update the docs; The following table represents the current and the desired classes and modules at the first moment. -| Current | Desired | Notes | -|-------------------------------------------------------|--------------------------------------------------|--------------------| -|`ActiveModelSerializers` and `ActiveModel::Serializer` | `ActiveModel::Serializers` | The main namespace | -| `ActiveModelSerializers.logger` | `ActiveModel::Serializers.logger` || -|`ActiveModelSerializers::Model` | `ActiveModel::Serializers::Model` || -|`ActiveModel::SerializableResource` | `ActiveModel::Serializers::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.config` | `ActiveModel::Serializers.config` || +| Current | Desired | Notes | +|--------------------------------------------------------|--------------------------------------------------|--------------------| +| `ActiveModelSerializers` and `ActiveModel::Serializer` | `ActiveModelSerializers` | The main namespace | +| `ActiveModelSerializers.logger` | `ActiveModelSerializers.logger` || +| `ActiveModelSerializers::Model` | `ActiveModelSerializers::Model` || +| `ActiveModel::SerializableResource` | `ActiveModelSerializers::SerializableResource` || +| `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` | `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 -This will be a breaking change, so all users serializers will be broken. -All PRs will need to rebase since the architeture will change a lot. +This will be a breaking change, so all users serializers will be broken after a +major bump. +All pull requests will need to rebase since the architeture will change a lot. # Alternatives We can keep the way it is, and keep in mind to not add another namespace as a 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 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.