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

refactored migrate command to handle multiple migrations paths #3273

Closed
wants to merge 4 commits into from
Closed

refactored migrate command to handle multiple migrations paths #3273

wants to merge 4 commits into from

Conversation

schmunk42
Copy link
Contributor

added migrationLookup property
fixes #384

@schmunk42
Copy link
Contributor Author

Note: Had to add alias to the migration table

@@ -97,7 +102,7 @@ public function options($actionId)
{
return array_merge(
parent::options($actionId),
['migrationPath', 'migrationTable', 'db'], // global for all actions
['migrationPath', 'migrationLookup', 'migrationTable', 'db'], // global for all actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this change affect the contents of the guide here: https://github.com/yiisoft/yii2/blob/master/docs/guide/console-migrate.md#use-command-line-options

If so, can you also adjust the guide too?

@qiangxue
Copy link
Member

I don't think we need the alias column. For issue #384, we only need to support setting migrationPath as an array which represents a list of search paths for migration classes.

@schmunk42
Copy link
Contributor Author

@qiangxue But the migrationPath is currently also used as the place to generate migrations. If we've an array here, how should we deal with this, maybe adding generateMigrationPath as an option?

@adamaltman Thanks for noticing. I'll update the guide when we've finished the implementation.

@qiangxue
Copy link
Member

When generating a migration, if there are multiple migrationPath, the user is required to enter a full path of the migration name.

@qiangxue
Copy link
Member

or perhaps he just needs to configure migrationPath to a single path when creating a new migration.

@schmunk42
Copy link
Contributor Author

I also think that would be the best option, but then in conjunction with Application::extensions where an extension could add a migration like so (in its bootstrap method):

$app->addMigrationPath('@vendor/myns/package/migrations');

Or should we specify this in the extras of the composer.json file?

Am I right, that there's no representation of @vendor/yiisoft/extensions.php yet or is Application::extensions NOT linked to that?

The migrate command would then merge the migration paths from Application::extensions and migrationPath from the command.

@qiangxue qiangxue mentioned this pull request Apr 28, 2014
@schmunk42
Copy link
Contributor Author

Broken code, too many changes.

@schmunk42 schmunk42 closed this Jul 2, 2014
@schmunk42 schmunk42 mentioned this pull request Jul 2, 2014
@schmunk42
Copy link
Contributor Author

Made this available as an extension: https://github.com/dmstr/yii2-migrate-command

@jacmoe
Copy link

jacmoe commented Feb 5, 2016

I really want this to be in Yii 2 - I do understand that this would break existing migration dbs, but I see no reason why it can't be implemented for fresh migrations?
This extension is one I always install - it enables me to take back control of migrations, whether or not they be mine or from third-party Yii extensions.
Excellent work @schmunk42 👍

@schmunk42
Copy link
Contributor Author

🙇 @jacmoe :)

@cebe
Copy link
Member

cebe commented Feb 6, 2016

This extension is one I always install

same here.

@jacmoe the issue #384 is still open. One day I will take the time to include it in the framework.

@jacmoe
Copy link

jacmoe commented Feb 6, 2016

Music in my ears @cebe 👍

@schmunk42
Copy link
Contributor Author

@cebe Here's a PR dmstr/yii2-migrate-command#9 which is based more on the current Yii 2.0 code. From my side, feel free to move the stuff to the core or contact me if you like...

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.

improve migrate command
5 participants