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

Move from ActiveModelSerializers::Model to ActiveModel::Serializer::Model #1296

Closed
wants to merge 2 commits into from

Conversation

maurogeorge
Copy link
Member

Hi guys,

Today the ActiveModelSerializers::Model is the only public API under this namespace.

To keep consistency move to ActiveModel::Serializer::Model.

Makes sense to you?

Thanks

…odel

The `ActiveModelSerializers::Model` is the only public API under this namespace.

To keep consistency move to `ActiveModel::Serializer::Model`.
[ci skip]
@bf4
Copy link
Member

bf4 commented Oct 24, 2015

I put it there on purpose, see (ongoing) discussion of the reasons in #1298

Holding this for now

@NullVoxPopuli
Copy link
Contributor

@maurogeorge I think the consensus was to use ActiveModel::Serializers::_____, so-- can you update this PR, accordingly?

@bf4
Copy link
Member

bf4 commented Nov 4, 2015

Consensus?

B mobile phone

On Nov 4, 2015, at 12:30 PM, L. Preston Sego III notifications@github.com wrote:

@maurogeorge I think the consensus was to use ActiveModel::Serializers::_____, so-- can you update this PR, accordingly?


Reply to this email directly or view it on GitHub.

@bf4
Copy link
Member

bf4 commented Nov 4, 2015

Should we rename the gem, too, then?

B mobile phone

On Nov 4, 2015, at 12:30 PM, L. Preston Sego III notifications@github.com wrote:

@maurogeorge I think the consensus was to use ActiveModel::Serializers::_____, so-- can you update this PR, accordingly?


Reply to this email directly or view it on GitHub.

@maurogeorge
Copy link
Member Author

@NullVoxPopuli thanks for the feedback. I will wait for a feedback of the team #1310 (comment) and when we have in the same page I will work on it.

@bf4
Copy link
Member

bf4 commented Nov 4, 2015

But the gem is not activemodel::serializers and rails has that namespace already

B mobile phone

On Nov 4, 2015, at 1:31 PM, L. Preston Sego III notifications@github.com wrote:

@bf4

#1310 (comment)
#1310 (comment) (kinda)
#1310 (comment)
The gem is already active_model_serializers, which I think fits well with ActiveModel::Serializers::___


Reply to this email directly or view it on GitHub.

@bf4
Copy link
Member

bf4 commented Nov 4, 2015

This can't be merged until #1310 is resolved

Changing the gem name is a pretty big deal, as is piggy-backing on existing rails namespaces

@NullVoxPopuli
Copy link
Contributor

the rails namespace is a module, which is fine.
as long as we keep the ...::Serializers a module, they can co-exist

@bf4
Copy link
Member

bf4 commented Jan 13, 2016

Closing as resolved in #1310

@bf4 bf4 closed this Jan 13, 2016
@maurogeorge
Copy link
Member Author

👍

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.

3 participants