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

Allow --repo option & --migration-dir for phx.gen.schema #4905

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

MikaAK
Copy link
Contributor

@MikaAK MikaAK commented Jul 29, 2022

This allows us to provide a --repo flag to mix phx.gen.schema

It seems support is already built in for this with Mix.Phoenix.Schema so I simply added it to the opts in the format it seems is expected and made it utilise that repo for the migration path as well

Through options i've also added a --migration_dir which is passed in and used instead of the generated repo path

If this is wanted lemme know and I'll add tests to get it merged

@MikaAK MikaAK force-pushed the repo-for-gen-schema branch 4 times, most recently from 792263c to 2a1c686 Compare August 4, 2022 03:53
@MikaAK MikaAK changed the title Allow --repo option for phx.gen.schema Allow --repo option & --migration-dir for phx.gen.schema Aug 4, 2022
@MikaAK MikaAK force-pushed the repo-for-gen-schema branch from 2a1c686 to 51acddd Compare August 4, 2022 03:58
Comment on lines 98 to 99
Generated migrations can be added to a specific `migration-dir` which sets the
migration folder path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Generated migrations can be added to a specific `migration-dir` which sets the
migration folder path
Generated migrations can be added to a specific `--migration-dir` which sets the
migration folder path:

## repo

Generated migration can use `repo` to set the migration repository
folder with option `--repo`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
folder with option `--repo`.
folder with option `--repo`:

@Gazler Gazler force-pushed the repo-for-gen-schema branch from 51acddd to 661cf51 Compare October 4, 2023 09:17
@Gazler Gazler requested a review from josevalim October 4, 2023 09:40
Comment on lines 212 to 222
case Map.new(opts) do
%{migration_dir: migration_dir} ->
migration_dir

%{repo: _} ->
repo_name = repo |> Module.split() |> List.last() |> Macro.underscore()
Mix.Phoenix.context_app_path(ctx_app, "priv/#{repo_name}/migrations/")

_ ->
Mix.Phoenix.context_app_path(ctx_app, "priv/repo/migrations/")
end
Copy link
Member

@josevalim josevalim Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure converting to a map for this is worth it, but this code is not perf critical anyway. But in case you want something else:

Suggested change
case Map.new(opts) do
%{migration_dir: migration_dir} ->
migration_dir
%{repo: _} ->
repo_name = repo |> Module.split() |> List.last() |> Macro.underscore()
Mix.Phoenix.context_app_path(ctx_app, "priv/#{repo_name}/migrations/")
_ ->
Mix.Phoenix.context_app_path(ctx_app, "priv/repo/migrations/")
end
cond do
migration_dir = opts[:migration_dir] ->
migration_dir
opts[:repo] ->
repo_name = repo |> Module.split() |> List.last() |> Macro.underscore()
Mix.Phoenix.context_app_path(ctx_app, "priv/#{repo_name}/migrations/")
true ->
Mix.Phoenix.context_app_path(ctx_app, "priv/repo/migrations/")
end

Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHIP IT

The `repo` and `migration-dir` options change the location of the
migrations directory, either to the specified directory or one that
matches the repo name (underscored).

The default is explicitly set to "repo" if neither option is set, to
ensure backwards compatability even if the repo has a different name
than the typical `MyApp.Repo`.
@Gazler Gazler force-pushed the repo-for-gen-schema branch from 661cf51 to 05c83f4 Compare October 4, 2023 18:56
@Gazler Gazler merged commit 09c6bc4 into phoenixframework:main Oct 4, 2023
4 checks passed
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.

3 participants