Skip to content
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

Dataloader.Ecto not compiled with Ecto 2.x #67

Closed
axelson opened this issue Feb 6, 2019 · 2 comments
Closed

Dataloader.Ecto not compiled with Ecto 2.x #67

axelson opened this issue Feb 6, 2019 · 2 comments

Comments

@axelson
Copy link

axelson commented Feb 6, 2019

Since commit 8a53e31 changes the optional dependency on ecto to ecto_sql, since my project is still on Ecto 2.x the optional dependency on ecto_sql has no effect. This means that dataloader may be compiled before ecto, which in turns means that the if Code.ensure_loaded?(Ecto) check fails in dataloader_ecto.ex and then my code breaks since it uses Dataloader.Ecto.

I'm not sure what the best course is for this. Some options/thoughts:

  1. List a breaking change that only Ecto 3.x is supported
  2. Split Dataloader.Ecto into a separate project that has a direct dependency on ecto
    a. This isn't ideal for me but it would've caused the error to appear earlier since I would have noticed ecto_sql being pulled in (or gotten a mix conflict)
  3. Revert the dependency change (this should be okay since everything that Dataloader.Ecto uses is actually from ecto and not ecto_sql
@benwilson512
Copy link
Contributor

@axelson yeah I think the only reason we did ecto_sql was for tests I think, can you do a PR where you move it to only: :test ?

axelson pushed a commit to axelson/dataloader that referenced this issue Feb 7, 2019
@axelson
Copy link
Author

axelson commented Feb 7, 2019

@benwilson512 created! #68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants