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

Move migrations under the Phinx\Migration namespace #168

Open
robmorgan opened this issue Dec 26, 2013 · 20 comments
Open

Move migrations under the Phinx\Migration namespace #168

robmorgan opened this issue Dec 26, 2013 · 20 comments
Milestone

Comments

@robmorgan
Copy link
Member

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
Copy link
Member Author

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

@ferodss
Copy link
Contributor

ferodss commented Dec 26, 2013

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

@robmorgan
Copy link
Member Author

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

@cyrusboadway
Copy link
Contributor

@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.

@evought
Copy link

evought commented Jan 13, 2015

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.

@t1gor
Copy link
Contributor

t1gor commented Jul 31, 2015

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

@arogachev
Copy link

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.

@rquadling
Copy link
Collaborator

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.

@wandersonwhcr
Copy link
Contributor

@rquadling +1

@josegonzalez
Copy link
Member

Would be great to do the same for Seeds :)

@garex
Copy link
Contributor

garex commented Aug 25, 2016

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.

@glensc
Copy link
Contributor

glensc commented May 12, 2017

#168 (comment) makes most sense,
#168 (comment) the parser solution is way overkill, it's like using tank to kill a fly.

@dereuromark
Copy link
Member

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

@dereuromark
Copy link
Member

Looks like this has been done.

@glensc
Copy link
Contributor

glensc commented Dec 17, 2017

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

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

@dereuromark dereuromark reopened this Dec 17, 2017
@chrisharrison
Copy link

@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.

@dereuromark
Copy link
Member

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

@dereuromark
Copy link
Member

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 ...

@EvilBMP
Copy link

EvilBMP commented Feb 12, 2018

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.

@dereuromark
Copy link
Member

We should check this for next major - 0.13 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests