Rails 8.0 #113
Rails 8.0 #113
Conversation
| union_values: Array, | ||
| union_operations: Array, | ||
| union_ordering_values: Array, | ||
| unionized_name: lambda { |klass| klass.arel_table.name } |
There was a problem hiding this comment.
Thanks for fixing this! this is an issue on rails 7.1 and 7.2 as well.
There is still a reference to @klass.arel_table on line 147 :
| union_values: DEFAULT_STORAGE_VALUE, | ||
| union_operations: DEFAULT_STORAGE_VALUE, | ||
| union_ordering_values: DEFAULT_STORAGE_VALUE, | ||
| unionized_name: proc { @table.name } |
There was a problem hiding this comment.
I did a simpler fix here which i think is better rather than relyin on an instance variable @table which can change overtime ( like @klass did )
chaadow@959c1e3
i think arel_table is more stable than @table or @klass because it's a very historic method, and is I belive more stable and can endure rails upgrades overtime, increasing the longevity of this gem 👍🏼
| @@ -141,7 +144,7 @@ def to_nice_union_sql(color = true) | |||
|
|
|||
| protected | |||
|
|
|||
| def build_unions(arel = @klass.arel_table) | |||
| def build_unions(arel = @table) | |||
There was a problem hiding this comment.
this method is only called once in the entire gem, i ve tried this default (basically calling build_unions without any arguments) and unfortunately it didnt work.
So maybe just remove the default value altogether?
There was a problem hiding this comment.
I cannot remember, but I had a valid reason for it at one point (years ago). I'll strip it since it should, in most cases, only be called with a provided arel reference.
|
Something that we'll need to figure out is how to deal with the following deprecations:
|
|
hi - i have some cycles and would be happy to help finish this pr (this blocks my workplace from upgrading to rails 8).
if i'm understanding correctly, is the ask here just to skip
would it be reasonable to just deprecate this method and aim to remove it in the next major version bump? |
No description provided.