Support new @raw_connection ivar name #309
Conversation
lib/semian/rails.rb
Outdated
| @@ -2,6 +2,8 @@ | |||
|
|
|||
| class ActiveRecord::ConnectionAdapters::AbstractAdapter | |||
| def semian_resource | |||
| @connection.semian_resource | |||
| # support for https://github.com/rails/rails/commit/d86fd6415c0dfce6fadb77e74696cf728e5eb76b | |||
| connection = instance_variable_defined?("@raw_connection") ? @raw_connection : @connection | |||
There was a problem hiding this comment.
I don't have an opinion, in fact I'm simply curious:
I wonder if explicitly using the Rails version to discriminate on the ivar has advantages? It certainly makes the intent more clear, in that it's for backwards compatibility. I suppose it's more fragile, though.
Curious what others think.
There was a problem hiding this comment.
I feel like that question comes often and I haven't heard strong preference on this. I believe last time we were discussing something similar we agreed that using explicit Rails version branching more often used but no obvious advantage/disadvantage
Also curious about other opinions!
There was a problem hiding this comment.
When it's not too complicated I prefer feature checking, especially when it's for yet unreleased versions, because I've seen changes like this be reverted in the past, so feature testing is more reliable.
lib/semian/rails.rb
Outdated
| @@ -2,6 +2,8 @@ | |||
|
|
|||
| class ActiveRecord::ConnectionAdapters::AbstractAdapter | |||
| def semian_resource | |||
| @connection.semian_resource | |||
| # support for https://github.com/rails/rails/commit/d86fd6415c0dfce6fadb77e74696cf728e5eb76b | |||
| connection = instance_variable_defined?("@raw_connection") ? @raw_connection : @connection | |||
There was a problem hiding this comment.
| connection = instance_variable_defined?("@raw_connection") ? @raw_connection : @connection | |
| connection = instance_variable_defined?(:@raw_connection) ? @raw_connection : @connection |
There was a problem hiding this comment.
Makes sense 👍 Changed
nvasilevski
force-pushed
the
support-rails-edge
branch
from
7384a76 to
fba9ca0
Compare
|
I may need to fix CI separately before merging this PR, I don't think failures are related to the change |
Rails change - rails/rails@d86fd64
Next version of Rails will use a different name for the
@connectionvariable andsemianneeds to support thisSince we can't rely on any version check at this moment so I used
instance_variable_definedto determine which variable to useIs there a better way to support both ivars?