namespacing migrations

[clphillips]

It doesn't appear that we can currently namespace our migrations. Here's why it would be good to have:

1. Prevents polluting global namespace.
2. Avoids collisions with other migrations a la Submodule migrations #9.
3. Allows migrations to pass PSR-2 style checks.

To accomplish this we'd need the ability to specify the namespace for each collection of migrations so they can be loaded correctly.

Thoughts?

[davedevelopment]

It's not particularly something I would use, but I'm not against it and would happily accept a contribution, provided it's optional.

Just FYI, the reasons I wouldn't use it:

• I don't consider them part of the app, don't load them during the course of running the application, so don't care about them polluting the global namespace.
• I've been using it for quite a while and never had a collision. I also consider migrations as disposable, so occasionally delete everything but the last x migrations
• Again, I don't really treat them as part of my app so wouldn't bother with style checks on them.

[clphillips]

I can agree with your use cases for typical apps, but what actually brought this up for me is a project that has user installable plugins which have their own independent migrations. When we perform an upgrade for the app we process migrations for the app as well as those for all installed plugins.

Since we process migrations as part of a larger upgrade process, we have more than just phpmig loaded. We also can't predict third party usage of migrations, so we can't be sure there won't be a collision with our app stack or migrations. Moreover, because we're dealing with so many more migrations due to the large pool of available plugins the likelihood of a collision is much higher.

After reading this article, I'm convinced this should be handled by extending and overriding AbstractCommand::bootstrapMigrations, though I can't see an easy way to do that.

It's trivial for PhpmigApplication::loadMigrations, which is what I intend to use. So I believe I can solve this for my use case. But a general solution is blocked by #66.

[davedevelopment]

Ugh, yeah, this lib isn't particularly well put together and does make some more complicated things hard to grind in.

I need to pencil some time in to spend on it regularly.

[clphillips]

Here's what I'd like to see:

1. Set default namespace in phpmig config (default to '' for global namespace)
2. If phpmig.migrations_path is '/migrations/', then every .php migration file in /migrations/ would belong to the default namespace.
3. Every subdirectory would append to the namespace. '/migrations/subdir/would concatsubdir` onto the namespace for all migrations within that directory.

Assume default namespace is set to 'My\Project'.

/migrations/
|_ 20150924000000_AddUsers.php (My\Project namespace)
|_ Modules/
  |_ 20150924010000_UpdateModule.php (My\Project\Modules namespace)

[ddelnano]

I am assuming this effort is over but I am looking to implement something like this. We I currently work I need namespacing for our migrations before we can merge a project that is wrapping up. Would a PR still be accepted? I am thinking that a good solution would be to pass in the expected namespace as an optional argument like so.

vendor/bin/phpmig migrate -b path/to/bootstrap.php --namespace="Foo\Bar"

or

vendor/bin/phpmig migrate -b path/to/bootstrap.php --n="Foo\Bar"

Would you accept a PR for that?

[ddelnano]

After taking a look to see what it might take, I think separating the migration logic out of the console commands and PhpmigApplication class would need to be the first step. This logic could then be shared and also so it would make the change less of a hack. I am willing to do both as long as you would be willing to accept a PR for it.

[davedevelopment]

@ddelnano I'm totally willing to accept a PR so long as it doesn't break existing functionality :)

[ddelnano]

Awesome should have something by the end of the weekend 👍

[clphillips]

See #66 for decoupling the migration logic from the console. Also note that the logic would need to be replaced in the Api as well.

[ddelnano]

Yea I saw that in your earlier comment and from looking through the codebase before have noticed the coupling. Thanks!