phinx icon indicating copy to clipboard operation
phinx copied to clipboard

Move migrations under the Phinx\Migration namespace

Open robmorgan opened this issue 11 years ago • 21 comments

In order to stop polluting the global namespace we will move migration classes under the Phinx\Migration namespace. This also conforms to PSR-2.

robmorgan avatar Dec 26 '13 15:12 robmorgan

Need to do a much better job of this.

Tasks

  • [ ] Fix Manager Classes to work with Namespaces
  • [ ] Fix Unit Tests
  • [ ] Find a solution for existing projects with migrations not under a namespace

robmorgan avatar Dec 26 '13 15:12 robmorgan

Maybe in 1.0 tag?! We broken compatibility in 1.0 release, no need much code to still work....

ferodss avatar Dec 26 '13 17:12 ferodss

@felipedjinn yes I think it will need to wait until 1.0 or 1.5. will break too much BC.

robmorgan avatar Dec 27 '13 04:12 robmorgan

@robmorgan In my mind, the ideal would allow arbitrary namespaces, either specified in the config and imported into the default template, or defined on a migration-by-migration basis, so that we can arrange the migrations in whichever directory/namespace structure makes sense for our application.

cyrusboadway avatar Feb 22 '14 00:02 cyrusboadway

If you go as @cyrusboadway suggests and allow a namespace to be specified in the config, then that will allow each project to put the migrations in their own vendor namespace and still preserve existing projects using the global namespace. New projects could default to Phinx\Migration . It would then become a backwards-compatible change.

Basically:

  • If the configuration value is defined, put/find migrations in the specified namespace.
  • The the configuration value does not exist, use the global namespace (current, backwards-compatible behavior).
  • When a new project is created (or transitions to phinx), have "phinx init" set the configuration value to Phinx\Migration in the generated phinx.yml. The user will have the opportunity to edit the value if desired.

evought avatar Jan 13 '15 19:01 evought

That's exactly what I'm looking for. Any update on the issue?

t1gor avatar Jul 31 '15 08:07 t1gor

I switched to Phinx from native framework migrations (I'm using Yii2) just to give it a try and see it in action. But this is for now is one of the biggest disadvantages for me. Any update on this?

I want namespace to be console\migrations\phinx. Maybe some workarounds exist? If I try to specify namespace, during migrate / rollback "No class found" expection is thrown.

arogachev avatar Nov 30 '15 03:11 arogachev

Some random thoughts.

The namespace will have to be added as a PSR-4 surely?

'Phinx\\Migration' => '%%PHINX_CONFIG_DIR%%/migrations'

The namespace will also need to be injected into the template so that custom templates and custom template generators will have the option of using the default namespace or not.

rquadling avatar Nov 30 '15 14:11 rquadling

@rquadling +1

wandersonwhcr avatar Dec 18 '15 12:12 wandersonwhcr

Would be great to do the same for Seeds :)

josegonzalez avatar Jan 07 '16 08:01 josegonzalez

Also namespaces should be different per path.

Currently we have single path and it's possible to use single namespace per this path in future.

But if we will use glob in path then we force to use same namespace everywhere.

May be implement class loading in another way? Let it parse migration and use class name from parsed file. I think https://github.com/nikic/PHP-Parser will do the job easily. And it will not be backward breaker.

garex avatar Aug 25 '16 09:08 garex

https://github.com/robmorgan/phinx/issues/168#issuecomment-69807486 makes most sense, https://github.com/robmorgan/phinx/issues/168#issuecomment-242325250 the parser solution is way overkill, it's like using tank to kill a fly.

glensc avatar May 12 '17 20:05 glensc

Is this ticket still valid? What is the plan of attack?

dereuromark avatar Oct 08 '17 16:10 dereuromark

Looks like this has been done.

dereuromark avatar Dec 16 '17 23:12 dereuromark

@dereuromark to me it seems it was commited and then reverted:

  • https://github.com/cakephp/phinx/commit/b141439c00e75549c7a0ed4bfe42b6fcd11f4fa8
  • https://github.com/cakephp/phinx/commit/c95aacc16a956786ba3cb9e1980004f2712c503c

so what exactly is done? please point to pull-request or commit.

glensc avatar Dec 17 '17 20:12 glensc

@rquadling It's a PSR-2 issue because a PHP file without a namespace is not PSR-2 compliant. My deployment script checks my codebase for conformance to PSR-2. The migrations are currently breaking that.

chrisharrison avatar Jan 12 '18 09:01 chrisharrison

Maybe we should allow for project namespaces here A project Foo could have Foo\Migration... per class then at least.

dereuromark avatar Jan 12 '18 10:01 dereuromark

For proper psr2 we also need to make the class names match the filenames.

// file M20170112080706OptionalSuffix.php
class M20170112080706OptionalSuffix ...

instead of

// file 20170112080706_optional_suffix.php
class OptionalSuffix ...

dereuromark avatar Jan 12 '18 14:01 dereuromark

Please allow custom namespaces like Vendor\Migrations and use class names like in doctrine migrations: class Version20180202110355 extends AbstractMigration with the corresponding filename Version20180202110355.php

Everything should be fine then. Even a migration suffix should be possible then - like @dereuromark suggested.

EvilBMP avatar Feb 12 '18 10:02 EvilBMP

We should check this for next major - 0.13 release.

dereuromark avatar Apr 11 '20 06:04 dereuromark