-
Notifications
You must be signed in to change notification settings - Fork 33
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 actionview an optional dependency #6
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manuca I understand that actionview can be an optional dependency because of this:
https://github.com/fastruby/next_rails/blob/master/lib/next_rails/gem_info.rb#L1-L5
And this: https://github.com/fastruby/next_rails/blob/master/lib/next_rails/gem_info.rb#L55-L59
But I have a question about activesupport.
next_rails.gemspec
Outdated
@@ -24,10 +24,11 @@ Gem::Specification.new do |spec| | |||
spec.require_paths = ["lib"] | |||
|
|||
spec.add_dependency "colorize", ">= 0.8.1" | |||
spec.add_dependency "activesupport", "< 6.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manuca How come this is necessary now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manuca How come this is necessary now?
@etagwerker This is the culprit:
https://github.com/manuca/next_rails/blob/make-actionview-optional/exe/bundle_report#L59
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manuca Weird. And where is that code getting called? What methods are we using from active_support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which to be honest I have no idea why we're depending on it as there are no acts_like?
calls anywhere as far as I've searched.
For reference this is acts_like.rb
:
https://github.com/rails/rails/blob/5-2-stable/activesupport/lib/active_support/core_ext/object/acts_like.rb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etagwerker I removed the activesupport
dependency. Specs pass and bundle_report
seems to work as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manuca Great, thanks! 👍
Resolves #5