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

Make railties an optional dependency #1922

Merged
merged 2 commits into from
Sep 14, 2016
Merged

Make railties an optional dependency #1922

merged 2 commits into from
Sep 14, 2016

Conversation

ggpasqualino
Copy link

Purpose

Make railties an optional dependency in order to use active_model_serializers in non rails projects again.

Related GitHub issues

#1921

@mention-bot
Copy link

@ggpasqualino, thanks for your PR! By analyzing the annotation information on this pull request, we identified @bf4, @dubadub and @steveklabnik to be potential reviewers

@richmolj
Copy link
Contributor

I'm in favor of this change. It looks like this PR by @bf4 moved us from rails as a development dependency to a runtime dependency, but reading the commit/PR this seems like it might have been unintended.

@bf4 is that correct?

(side note: if we go in the direction of deprecating ActiveModelSerializers::Model activemodel could move to a development dependency as well, I think)

@richmolj
Copy link
Contributor

Also, just a heads up we are already only requiring the railtie if Rails is defined so no worries there.

@ggpasqualino even though this is a one-line change, I could see an entry in the changelog being helpful.

@ggpasqualino
Copy link
Author

@richmolj thank you. I will check the changelog and update the PR.

Sorry about this mess. I didn't realize that when I created #1921 because I was looking at the wrong version of the file, then I realised that in master you are already only requiring the railtie if Rails is defined and created this Pull Request.

@@ -31,7 +31,7 @@ Gem::Specification.new do |spec|
# 'rack'
# 'rack-test', '~> 0.6.2'

spec.add_runtime_dependency 'railties', rails_versions
spec.add_development_dependency 'railties', rails_versions
Copy link
Member

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants