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

Adds --no-default-migration option diesel_cli #4184

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

lgrosz
Copy link
Contributor

@lgrosz lgrosz commented Aug 19, 2024

Adds --no-default-migration option diesel_cli to disable the generation of the default migration on setup.

This also tweaks the behavior of the db subcommands so they never generate the migration.

@weiznich weiznich requested a review from a team August 19, 2024 05:34
Copy link
Member

@weiznich weiznich left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. I've added two comments about the reset and setup database command, which should also use the new flag instead of changing the default behaviour.

diesel_cli/src/main.rs Outdated Show resolved Hide resolved
diesel_cli/src/database.rs Outdated Show resolved Hide resolved
@weiznich weiznich force-pushed the optional-default-migration branch from 2e8bb1c to 7be5767 Compare August 21, 2024 06:33
@lgrosz
Copy link
Contributor Author

lgrosz commented Aug 21, 2024

Thanks for working on this. I've added two comments about the reset and setup database command, which should also use the new flag instead of changing the default behaviour.

Sounds good. I can make those changes. Out of curiosity, does it make sense to be creating any migrations during any of the diesel database sub-commands? I was under the impression those commands would setup the database with the existing migrations while diesel setup would be used to "setup" the project, encompassing creating the default migration.

Either way, I'm aware such a behavior change wouldn't belong in a quick patch like this.

@lgrosz lgrosz force-pushed the optional-default-migration branch 2 times, most recently from 63ea1aa to 32b31b1 Compare August 22, 2024 04:56
Adds --no-default-migration option diesel_cli to disable the generation
of the default migration on setup where it might happen.
@lgrosz lgrosz force-pushed the optional-default-migration branch from 32b31b1 to 2ec224e Compare August 22, 2024 04:57
@lgrosz
Copy link
Contributor Author

lgrosz commented Aug 22, 2024

Sorry for all the updates.. I'm done now.

@weiznich
Copy link
Member

I was under the impression those commands would setup the database with the existing migrations while diesel setup would be used to "setup" the project, encompassing creating the default migration.

diesel setup setups your project, which not only includes the database, but also the local folder structure. That includes the default migrations + a default diesel.toml file.

@weiznich weiznich added this pull request to the merge queue Aug 23, 2024
Merged via the queue into diesel-rs:master with commit bfe9989 Aug 23, 2024
48 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.

2 participants