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

Support Rails 3.2 #2

Merged
merged 1 commit into from
Apr 25, 2015
Merged

Support Rails 3.2 #2

merged 1 commit into from
Apr 25, 2015

Conversation

tfausak
Copy link
Contributor

@tfausak tfausak commented Apr 24, 2015

The only thing this library uses from Rails 4 is the new Object#deep_dup behavior. This pull request backports that behavior to support Rails 3.2.

spec.add_dependency "activesupport"

spec.add_development_dependency "bundler", "~> 1.7"
spec.add_development_dependency "minitest", "~> 5.6"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was necessary because minitest does not get pulled in as a transitive dependency with activemodel 3.2.

@morgoth
Copy link
Member

morgoth commented Apr 25, 2015

I can see that there are more problems on 3.2.x:

# 3.2.0
bundle exec rake
/home/wojtek/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activesupport-3.2.0/lib/active_support/core_ext/module/deprecation.rb:7:in `deprecate': uninitialized constant ActiveSupport::Deprecation (NameError)
    from /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activemodel-3.2.0/lib/active_model/naming.rb:13:in `<class:Name>'
    from /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activemodel-3.2.0/lib/active_model/naming.rb:7:in `<module:ActiveModel>'
    from /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activemodel-3.2.0/lib/active_model/naming.rb:6:in `<top (required)>'
    from /home/wojtek/Projects/active_model-errors_details/test/test_errors_details.rb:5:in `require'
    from /home/wojtek/Projects/active_model-errors_details/test/test_errors_details.rb:5:in `<top (required)>'
    from /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/2.2.0/rake/rake_test_loader.rb:10:in `require'
    from /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/2.2.0/rake/rake_test_loader.rb:10:in `block (2 levels) in <main>'
    from /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/2.2.0/rake/rake_test_loader.rb:9:in `each'
    from /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/2.2.0/rake/rake_test_loader.rb:9:in `block in <main>'
    from /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/2.2.0/rake/rake_test_loader.rb:4:in `select'
    from /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/2.2.0/rake/rake_test_loader.rb:4:in `<main>'
rake aborted!
# 3.2.1 upto 3.2.12
bundle exec rake
MiniTest::Unit::TestCase is now Minitest::Test. From /home/wojtek/Projects/active_model-errors_details/test/test_errors_details.rb:7:in `<top (required)>'
Run options: --seed 31318

# Running:

...E...

Finished in 0.007318s, 956.5743 runs/s, 1366.5347 assertions/s.

  1) Error:
TestErrorsDetails#test_dup_duplicates_details:
NoMethodError: private method `initialize_dup' called for #<ActiveModel::Errors:0x007fa385198ae0>
    /home/wojtek/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activemodel-3.2.1/lib/active_model/errors.rb:90:in `dup'
    /home/wojtek/Projects/active_model-errors_details/test/test_errors_details.rb:69:in `test_dup_duplicates_details'

It was fixed in 3.2.13 in rails/rails#8405

I'll set minimum required version to 3.2.13 - I hope this is fine for you.
I would also like to avoid maintaining 3.2.x compatibility in a long term.

@morgoth morgoth merged commit b3ff687 into cowbell:master Apr 25, 2015
@tfausak
Copy link
Contributor Author

tfausak commented Apr 25, 2015

Oops, I should have tested with 3.2.0. I want to use this library with orgsync/active_interaction, which supports Rails 3.2.0. We will support that at least until Rails 5 is released. If you do not want to support Rails 3.2.x, we can fork this gem to add support.

@morgoth
Copy link
Member

morgoth commented Apr 25, 2015

After your changes it works with 3.2, so no need to fork.
I think that setting 3.2.13 as minimum dependency shouldn't be a problem. Latest 3.2 series is 3.2.21 so there were lot of releases.
If someone would like to use new version of active_interaction he shouldn't have any problem with upgrading rails itself.

@tfausak tfausak deleted the rails-3-2 branch April 28, 2015 17:19
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.

2 participants