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

Correct migration step in phx.gen.auth docs #5937

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

jrko
Copy link
Contributor

@jrko jrko commented Sep 24, 2024

After running mix phx.gen.auth you are prompted to run mix deps.get followed by mix ecto.migrate.

The docs reference mix ecto.setup however, which seems to be deprecated in favor of mix ecto.create, neither of which are correct in this context, assuming a basic Phoenix app is up and running prior to adding auth.

This PR corrects the docs, replacing mix ecto.setup with mix ecto.migrate along with an appropriate description of the step.

It also adds the assumption that the reader has gone through the introductory guides and has a running Phoenix app, as well as calling out the addition of the Register and Log in links added to the default page.

I also took the liberty to remove some extraneous language about umbrella app root locations, which seemed unnecessary in the context of learning auth basics.

After running `mix phx.gen.auth` you are prompted to run `mix deps.get` followed by `mix ecto.migrate`.

The docs reference `mix ecto.setup` however, which seems to be deprecated in favor of `mix ecto.create`, neither of which are correct in this context, assuming a basic Phoenix app is up and running prior to adding auth.

This PR corrects the docs, replacing `mix ecto.setup` with `mix ecto.migrate` along with an appropriate description of the step.

It also adds the assumption that the reader has gone through the introductory guides and has a running Phoenix app, as well as calling out the addition of the `Register` and `Log in` links added to the default page. 

I also took the liberty to remove some extraneous language about umbrella app root locations, which seemed unnecessary in the context of learning auth basics.
@josevalim
Copy link
Member

ecto.setup is generated as part of new applications, so we should continue to refer to it:

"ecto.setup": ["ecto.create", "ecto.migrate", "run priv/repo/seeds.exs"],
.

If your app does not have it, maybe it was generated in older versions or removed?

@jrko
Copy link
Contributor Author

jrko commented Oct 25, 2024

Aah, I do indeed have ecto.setup (using Elixir 1.14.0, Erlang/OTP 24, Phoenix 1.7.14). I think my confusion came from a couple places:

  1. The console output of mix phx.new and the Up and Running docs both reference ecto.create.
  2. Searching Ecto HexDocs for ecto.setup returns no results, but ecto.create does.

I see now that ecto.setup is a valid alias in Phoenix, though it still seems a bit awkward to have the console output of phx.gen.auth reference ecto.migrate as a next step, while the auth docs reference ecto.setup.

Anyway, feel free to close the PR if you like. I was able to work through the confusion without too much fuss, but thought I might be able to streamline the docs a bit for future newcomers, like myself!

@josevalim josevalim merged commit 7586cbe into phoenixframework:main Oct 25, 2024
6 of 7 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

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

Successfully merging this pull request may close these issues.

2 participants