-
Notifications
You must be signed in to change notification settings - Fork 370
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
Rails 6.1 Compatibility #365
Conversation
…dule#module_parent` Rails 6 deprecated `Module#parent` and friends in rails/rails#34051, replacing them with a `module_*`-prefix naming scheme. This checks to see if `module_parent` is defined, using it if it's available.
@kerrizor I think this is ready for review now :) |
Would love to have this merged! I've got the fork's master branch pointed to in my Gemfile, thanks for this fix @andrew-newell ! |
I also would love to have this merged and released! I just discovered the library, and patched my local installation. This should fix #363 |
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.
Wondering why I can approve this PR 😅 , but I will give it a try.
I like your efforts, unfortunately I don't think you or I have the permissions to merge lol |
Just trying to get some attention from people that can 😂 |
Same reason why I commented! |
I'm on 6.1.0 and can confirm that this fixes the error. Thank you. |
Ah thanks for the PR, a quick hack here until someone responds:
copy the commit content from this file here to below file:
|
🤖 💡 Just wanting to keep this bumped because I don't want to see it get auto-closed or anything! |
@andrew-newell Looks good! I'm honestly surprised the tests continue to pass for 4.2, but it's probably time to EOL support for that. CI comes back 🍏 I'll merge and push a new point release out.. Thank you for the contribution! 🏆 |
v1.6.1 pushed to Ruby Gems and should be available. Any issues, please open a new issue! |
I'll be completely honest, this fix works, but I haven't spent the time to guarantee this is the "right" way to update this library for Rails 6.1 compatibility. Due to the way the tests fiddle with the innards of ActiveRecord, it seems like this library is only a few more major Rails releases away from needing its tests rewritten.
I saw the comments on #345 by @gee-forr and several others, hopefully I'll be able to get the tests in travis happy if/when they fail.