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

feat: allow to configure migrations configuration files #686

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

MatTheCat
Copy link
Contributor

@MatTheCat MatTheCat commented Aug 9, 2024

This PR introduces a new orm.reset.migrations.configurations configuration node allowing a schema reset to run doctrine:migrations:migrate for each given configuration. This is useful because using doctrine/migrations, you can only target one single connection or entity manager per configuration file.

Fixes #685

Copy link
Member

@nikophil nikophil left a comment

Choose a reason for hiding this comment

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

Please add a test for this 🙏

src/ORM/AbstractORMPersistenceStrategy.php Show resolved Hide resolved
src/ZenstruckFoundryBundle.php Show resolved Hide resolved
@MatTheCat
Copy link
Contributor Author

@nikophil not sure how to add a test; my attempt fails and I don’t understand why 🤔

@nikophil
Copy link
Member

nikophil commented Sep 1, 2024

hum this error Can't create database 'cms'; database exists already drove me crazy few days ago, I need to deep dive into it... we're doing some weird stuff with migrations and with this "cms" schema.

Another thing is that I think we should not have a full test suite with "migration with configuration", or even for "migration" at all. But this goes way beyond the purpose of this PR.

What about this: please, remove all the modification about the CI (sorry for your wasted time around it), and let's merge your PR. I've manually tested your branch on a project of mine and can confirm it is working. I will then work on a cleaner solution for this whole thing in the tests

@MatTheCat
Copy link
Contributor Author

No worries, I procrastinated a lot on this one..!

Reverted every test-related change 🧹

Copy link
Member

@nikophil nikophil left a comment

Choose a reason for hiding this comment

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

LGTM

@nikophil nikophil merged commit 6482357 into zenstruck:2.x Sep 5, 2024
30 checks passed
@MatTheCat MatTheCat deleted the ticket_685 branch September 29, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Allow to reset many databases in migrate mode
2 participants