-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add CI for ruby3.0 #70
Conversation
lib/activerecord-bitemporal.rb
Outdated
def initialize_dup(other) | ||
# MEMO: Do not copy `swapped_id` | ||
if instance_variable_defined?(:@_swapped_id) | ||
remove_instance_variable(:@_swapped_id) | ||
end | ||
|
||
super | ||
end |
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.
Doesn't it break when #initialize_dup
is defined in user code?
# User code
class User < ApplicationRecord
include
def initialize_dup
# other implemented
end
end
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.
Also, doesn't this implementation also change the behavior of code that calls #dup
elsewhere?
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.
Doesn't it break when #initialize_dup is defined in user code?
It isn't problem because it is the same as #dup
. Normally user shouldn't define initialize_dup
without super.
Also, doesn't this implementation also change the behavior of code that calls #dup elsewhere?
Right. I just thought that #dup
should clear instance scoped variables but it is not necessary as you said.
I just resolved this issue by replacing refine Object
with refine ActiveRecord::Base
. This way is the most little changes.
Thanks for PR :)
Yes, I believe this is a bug in Ruby (or other gem). see:
|
Great!! Thank!! |
Don't refine classes that it isn't necessary. As a technical issue, overriding Object#dup causes a problem of stopping `ActiveRecord::Base.establish_connection` in Ruby 3.0. I will try to continue to investigate this issue.
@osyo-manga done :) |
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.
Thanks :)
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.
🥳
Add ruby 3.0 support 🎉
>= 6.1.0
#dup
methodwhy I changed refinements for
#dup
In the case of using ruby3.0 & rails6.1.0, when you call
ActiveRecord::Base.establish_connection
, the process will freeze and stop working. I investigated this issue, I found that refinements for#dup
causes this problem.I think that Ruby is not tested well enough for refinements because the override
#dup
with refinements is not recommended way.related: https://bugs.ruby-lang.org/issues/17494