-
Notifications
You must be signed in to change notification settings - Fork 123
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
Rails5 Upgrade #86
base: master
Are you sure you want to change the base?
Rails5 Upgrade #86
Conversation
As of ActiveRecord 5.0 monkey-patchings initiated by ActiveUUID::Patches.apply! do not modify Mysql2Adapter and/or SQLite3Adapter of ActiveRecord because they are not loaded by the time the apply! method is invoked. To load patches properly on loading time, we override ActiveRecord::Base.establish_connection method to load the patches immediately after connections are established.
As loading order of activeuuid and connection establishment in spec_helper has been switched.
Rails 5 + MySQL, SQLite support
Previous spec failures were from changed behaviour of described_class on RSpec 3.x, which referenced the innermost `context' instead of `describe'.
Issue on rails/rails#25125 causes an infinite loop on rails < 5.0
Update dependencies and drop EOL-ed rubies/rails
I'd love to be able to merge this. Could you get the tests passing? |
@jashmenn I think we'd better to merge my fork first, as this PR is based on that and all tests pass on it. You can take a look on https://github.com/inbeom/activeuuid. Things changed:
May I open another PR? |
Please merge the fork by inbeom. We've been using the rails5 fix in production for months. |
Where has this been left? |
@aren55555 You might want to try my fork. It passes the tests with Rails 5 environment. |
@inbeom Without looking at all the commits; what is the difference between @jlsookiki's fork (aka this PR) and your fork? How can I help to get this merged? What tests are failing? Where can I lend a hand? |
If we can get the tests passing I'm happy to merge! |
@jashmenn #85, what this one is based on, passes tests on the all environments but the obsolete ones. Can you consider merging it first? I think now almost no one expects gems to pass their tests on Ruby 2.0, RBX, JRuby while maintaining compatibility with the most recent versions of Ruby. Plus, Travis lacks support of recent versions of RBX and JRuby. There are some more commits pushed to my fork regarding the narrowing down of supported environments, making its status green. |
@jlsookiki and @inbeom I've added you as collaborators, feel free to merge when you believe this is ready |
@jashmenn Thanks. I will catch up some open issues and prepare to put my hands on the maintenance of this gem. |
@inbeom @jlsookiki do you still plan to merge these PRs? |
Piggy backs off @inbeom's PR - fixes the test failures ( I think) and also does some refactoring to remove final deprecations from
alias_method_chain
calls