-
-
Notifications
You must be signed in to change notification settings - Fork 389
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
Support multiple versions in ExecuteCommand #891
Conversation
8b1d5cc
to
e6646de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the changes for multiple versions. Looks cool on first sight. Can you please also create a test for multiple versions in the version array in MigrationPlanCalculatorTest
, similar to testPlanForVersionsWhenNoMigrations
but with more than only C?
@SenseException good catch. I have also added more tests to make sure migrations are properly sorted (keeping sort order and not the order provided by the user via command line) |
Any other feedback on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor doc feedback, functionality looks good. 👍
lib/Doctrine/Migrations/Tools/Console/Command/ExecuteCommand.php
Outdated
Show resolved
Hide resolved
7f1e435
to
c820fa4
Compare
lib/Doctrine/Migrations/Tools/Console/Command/ExecuteCommand.php
Outdated
Show resolved
Hide resolved
bf89d43
to
4c7a71b
Compare
@alcaeus @SenseException the changes you asked are done. can you please check again/approve this PR? |
4c7a71b
to
7b9dccd
Compare
Rebased on top of the latest master branch. |
Thanks @goetas ! |
Summary
Implementation of #796 for the 3.x branch