search

elixir-ecto / ecto

A toolkit for data mapping and language integrated query.
https://hexdocs.pm/ecto
Apache License 2.0
6.14k stars 1.43k forks source link

Changelog update for Mix.Ecto.migrations_path/1 #2763

Closed kipcole9 closed 5 years ago

kipcole9 commented 5 years ago

Changelog update for backward incompatible API change

The function migrations_path/1 has moved from Mix.Ecto to Ecto.Migrator. This is understandable since ecto 3 shouldn't have database knowledge as that's moved to ecto_sql.

Suggested update to the changelog

Backwards incompatible changes

josevalim commented 5 years ago

Thanks @kipcole9! Note that Mix.Ecto was private, which is why we didn't list this change in the CHANGELOG.

kipcole9 commented 5 years ago

Thanks José. May I assume Ecto.Migrator.migrations_path/1 is public? I see nothing in the module to suggest otherwise and being consistent about where to generate migrations for libs is very helpful.

I try to use only public API's but seems I regularly step over the line unknowingly. I have interpreted an @doc on a def function as public - is that the wrong assumption?

josevalim commented 5 years ago

Oh, i see. If you have @moduledoc false in a module, then all functions in that module are private, regardless of @doc. If in doubt, you can consult the online docs. Ecto.Migrator.migrations_path is pubic though :+1:.

nathany commented 5 years ago

How should a library find the migrations_path for Ecto 2.x and 3.x?

See https://github.com/ueberauth/guardian_db/issues/93

josevalim commented 5 years ago

The suggested Ecto.Migrator.migration_path should work on both. --

José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D

nathany commented 5 years ago

Doesn't work for me (in ecto 2.2)

josevalim commented 5 years ago

Ah, you are correct, sorry.

i think you can check the Ecto version or if use code.ensure_loaded? and function_exported? to see if the migrator function exists accordingly. --

José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D

nathany commented 5 years ago

Thanks @josevalim.

Example of resolving this in another library (as pointed out on Slack): https://github.com/kipcole9/money/commit/c2766dece7f241f25b86999876d756d7b7d7a4be