arbor icon indicating copy to clipboard operation
arbor copied to clipboard

Ecto struct_fields

Open wwww-wwww opened this issue 2 years ago • 3 comments

https://github.com/coryodaniel/arbor/blob/1d3c610f4c1cc334af332aa11516cf7d543d2209/lib/arbor/tree.ex#L39

It is now :ecto_struct_fields in Ecto 3.8

Although switching to :ecto_struct_fields would also imply ecto >= 3.8 and elixir ~> 1.10

wwww-wwww avatar May 08 '22 14:05 wwww-wwww

What if we try to fetch from :ecto_struct_fields and, if nil, we try to fetch from :struct_fields?

We would keep compatibility with previous versions of Ecto while also supporting >= 3.8 (and not requiring Elixir ~> 1.10 if they are using Ecto <= 3.8).

Would that work? Seems like a simple fix to make.

Something like this:

struct_fields = Module.get_attribute(definition, :ecto_struct_fields) || Module.get_attribute(definition, :struct_fields)

Or, more explicitly:

struct_fields =
  if Module.has_attribute?(definition, :ecto_struct_fields) do
    # Ecto >= 3.8
    Module.get_attribute(definition, :ecto_struct_fields)
  else
    Module.get_attribute(definition, :struct_fields)
  end

If that doesn't work by some reason I'm not seeing, the README already has instructions for Ecto 2 and Ecto 3. Maybe having a version bump for >= Ecto 3.8 wouldn't be so bad...

What do you folks think, @wwww-wwww and @coryodaniel? Our project requires >= Ecto 3.8 due to another dependency and I'm currently vendoring arbor with this fix.

If we decide on an approach, I can make a PR so we can fix this.

durub avatar May 10 '22 14:05 durub

I think that sounds like a good approach. Are there any other 3.8 issues you’ve run into?

coryodaniel avatar May 10 '22 15:05 coryodaniel

I haven't run into any issues yet.

wwww-wwww avatar May 10 '22 23:05 wwww-wwww