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

ADD: down method to migration file #272

Closed
wants to merge 1 commit into from
Closed

Conversation

mrmmg
Copy link

@mrmmg mrmmg commented May 12, 2024

Hi,

I recently installed this package and when I run the php artisan migrate:refresh command, Laravel throws an exception that the settings table already exists. The issue is that the migration does not include a down method, so the settings table is not removed in migration refresh. So, I added the down method to it.

Let me know if you'd like me to suggest any further changes!

@rubenvanassche
Copy link
Member

We don't use down migrations in any package / project.

99.9% of the time you are not gonna use a down migration and you’re probably never testing them. Just write an up migration if you want to drop a column again. 👍

@mrmmg
Copy link
Author

mrmmg commented May 13, 2024

We don't use down migrations in any package / project.

99.9% of the time you are not gonna use a down migration and you’re probably never testing them. Just write an up migration if you want to drop a column again. 👍

That's incorrect! In Laravel Package, you typically publish your migration files to the database/migrations folder. Based on your thought, if I want to define another migration, I would need to create a dummy migration and change the file name, since Laravel loads migration files based on their names. However, I believe this approach is not ideal for removing the settings table. Am I correct?"

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