Ecto 3.0 support #301
Ecto 3.0 support #301
Conversation
|
Thanks for looking into this @zillou. I don't think we have any particular plans for Ecto 3 in terms of new features or anything like that. But soon after it is out, we can do what you're doing here and go with it. |
mix.exs
Outdated
| @@ -36,8 +36,9 @@ defmodule ExMachina.Mixfile do | |||
| [ | |||
| {:ex_doc, "~> 0.14", only: :dev}, | |||
| {:earmark, ">= 0.0.0", only: :dev}, | |||
| {:ecto, "~> 2.1", optional: true}, | |||
| {:postgrex, ">= 0.0.0", only: [:test]}, | |||
| {:ecto_sql, "~> 3.0.0-pre", optional: true}, | |||
There was a problem hiding this comment.
ecto ~> 2.1 or ~> 3.0 would work too
There was a problem hiding this comment.
agreed, that would definitely be preferred and it matches phoenix_ecto: https://github.com/phoenixframework/phoenix_ecto/blob/v3.5.0/mix.exs#L40
mix.exs
Outdated
| {:postgrex, ">= 0.0.0", only: [:test]}, | ||
| {:ecto_sql, "~> 3.0.0-pre", optional: true}, | ||
| {:jason, "~> 1.0", optional: true}, | ||
| {:postgrex, "~> 0.14.0-pre", only: :test}, |
|
Thank you @archseer, @wojtekmach. |
mix.exs
Outdated
| {:postgrex, ">= 0.0.0", only: [:test]}, | ||
| {:ecto, "~> 2.2 or ~> 3.0", optional: true}, | ||
| {:jason, "~> 1.0", optional: true}, | ||
| {:ecto_sql, "~> 2.2 or ~> 3.0-pre", only: :test}, |
There was a problem hiding this comment.
ecto_sql 3.0 got released, so you can change 3.0-pre to 3.0.
|
looks good to me |
| {:ecto, "~> 2.1", optional: true}, | ||
| {:postgrex, ">= 0.0.0", only: [:test]}, | ||
| {:ecto, "~> 2.2 or ~> 3.0", optional: true}, | ||
| {:jason, "~> 1.0", optional: true}, |
There was a problem hiding this comment.
Do we need to include jason as part of upgrading to 3.0? If so, is it safe to remove poison?
There was a problem hiding this comment.
Looking at code, Poison is only used in docs so I'd suggest to remove it from mix.exs and mix.lock and optionally promote Jason as it's recommended by Ecto & Phoenix.
| {:ecto, "~> 2.2 or ~> 3.0", optional: true}, | ||
| {:jason, "~> 1.0", optional: true}, | ||
| {:ecto_sql, "~> 2.2 or ~> 3.0", only: :test}, | ||
| {:postgrex, "~> 0.14.0", only: :test}, |
There was a problem hiding this comment.
Do we need to update postgrex or is it fine to leave it as >= 0.0.0?
There was a problem hiding this comment.
>= 0.0.0 should work fine, ecto_sql will already lock it to whatever version it needs.
https://github.com/elixir-ecto/ecto_sql/blob/a11efc4e4d3a6a9025ca96894cb9f708a121d0fd/mix.exs#L52
| @@ -36,8 +36,10 @@ defmodule ExMachina.Mixfile do | |||
| [ | |||
| {:ex_doc, "~> 0.14", only: :dev}, | |||
| {:earmark, ">= 0.0.0", only: :dev}, | |||
| {:ecto, "~> 2.1", optional: true}, | |||
| {:postgrex, ">= 0.0.0", only: [:test]}, | |||
| {:ecto, "~> 2.2 or ~> 3.0", optional: true}, | |||
There was a problem hiding this comment.
Do we need to keep this, or should we only be using {:ecto_sql, "~> 3.0"}? I think it was needed when working with release candidates, but now it should be fine to remove (as far as I understand).
I'm getting that idea from reading this blog post,
If you are using Ecto with a SQL database, migrating to Ecto 3.0 will be very straight-forward. Instead of:
{:ecto, "~> 2.2"}
You should list:
{:ecto_sql, "~> 3.0"}
There was a problem hiding this comment.
Actually, I was not quite sure about which library we need to use in ExMachina. I just did a quick eyeballing of the codebase, I didn't find any code related to ecto_sql, so I chose ecto here.
I'll update the deps list if we do need ecto_sql.
There was a problem hiding this comment.
Will there be any chance that ExMachina is used in a project without a SQL database, with the same reason of the splition of Ecto.SQL?
There was a problem hiding this comment.
I would keep ecto as {:ecto, "~> 3.0"} and ecto_sql as {:ecto_sql, "~> 3.0", optional: true}, since you can technically use ex_machina to only build structs and not write to the DB. That said, I've had the same question in a few other dependencies where it's not as obvious.
| {:postgrex, ">= 0.0.0", only: [:test]}, | ||
| {:ecto, "~> 2.2 or ~> 3.0", optional: true}, | ||
| {:jason, "~> 1.0", optional: true}, | ||
| {:ecto_sql, "~> 2.2 or ~> 3.0", only: :test}, |
There was a problem hiding this comment.
Should ecto_sql have an optional: true here since ex_machina can be used without ecto?
There was a problem hiding this comment.
It's limited to only: test so it should work the same way
| use Ecto.Repo, otp_app: :ex_machina | ||
| use Ecto.Repo, | ||
| otp_app: :ex_machina, | ||
| adapter: Ecto.Adapters.Postgres |
|
I've just started a new project with Ecto 3 a few days ago and now I'm needing some factories to speed up my typing ... Thanks a lot in advance for this and I hope it can be merged soon! |
|
@zillou thanks again for these changes! I took another look, and I think they're good. I had mentioned getting rid of poison, but I can do that in a separate commit. |
Looks like 3.0 is supported as of #301
Not sure about the plan for Ecto 3.0 support. But I tested against ecto_sql 3.0.0-rc1, and everything is working perfectly.