Doctrine Migrations #62
Conversation
ghost
mentioned this pull request
| dir_name: '%kernel.project_dir%/DoctrineMigrations' | ||
| namespace: App\Migrations | ||
| table_name: migration_versions | ||
| name: Application Migrations |
There was a problem hiding this comment.
please remove the table_name and the name from this config, as requested in #57. We don't need to configure them explicitly in the project.
I would even remove the explicit configuration of the namespace here, keeping the default one. I don't see any reason to change it here (one could argue that you want to make the namespace fit your app, but your config will not make it compatible with PSR-4 anyway so this is a bad argument. And when using advanced feature to organize files per date instead of having a single folder with hundred of migrations, you cannot make code inside this folder fit PSR-4 at all, so I would not even try it).
This would then leave only the dir_name config (which is the one we really need due to the new Flex structure)
|
@fabpot there was some discussion in the past about forcing bundles to use their DI alias for the naming of the YAML config file they add in |
| @@ -0,0 +1,2 @@ | |||
| doctrine_migrations: | |||
| dir_name: '%kernel.project_dir%/DoctrineMigrations' | |||
There was a problem hiding this comment.
All directory names are using the underscore notation, having a DoctrineMigrations directory at the root of the project looks weird. What about something like '%kernel.project_dir%/etc/migrations' instead?
There was a problem hiding this comment.
I would also add etc/migrations/.gitkeep to the PR so that the directory is automatically created when installing the bundle.
There was a problem hiding this comment.
@fabpot Should src/migrations not be src/Migrations. Because the Entity folder has a capital letter to in the directory. Let me know, I've added it with a Capital letter for now.
| @@ -0,0 +1,2 @@ | |||
| doctrine_migrations: | |||
| dir_name: '%kernel.project_dir%/DoctrineMigrations' | |||
There was a problem hiding this comment.
in #57, we said that migrations should live inside src (as they are code), not at the root
There was a problem hiding this comment.
@fabpot do you think migration classes should go in src or etc ? Your previous comment suggests etc, but migrations are code.
There was a problem hiding this comment.
Indeed, that should be src/, not etc/.
There was a problem hiding this comment.
Would it make more sense to use singular Migration instead of Migrations for the namespace?
There was a problem hiding this comment.
@rybakit We only changed the default directory for now. If you want to customize the namespace or another setting of the bundle I suggest you take a look at the options at http://symfony.com/doc/current/bundles/DoctrineMigrationsBundle/index.html#configuration
/cc @fabpot @stof @javiereguiluz Might be a idea to add "env": {} to the manifest file with a link to doc/current/bundles/DoctrineMigrationsBundle/?
There was a problem hiding this comment.
@Ricknox Sorry, I meant if singular src/Migration as the default directory would be more appropriate here (as the migrations are php classes, and most of the libraries use singular names in their namespace to describe the domain, fe https://github.com/symfony/recipes/blob/master/doctrine/doctrine-bundle/1.6/etc/packages/doctrine.yaml#L12)
ghost
dismissed
|
@stof is right, you rename |
|
Thank you @Ricknox. |
This PR was merged into the master branch. Discussion ---------- Move etc/ to config/ and web/ to public/ | Q | A | ------------- | --- | License | MIT Must be merged just after symfony/flex#122 Commits ------- 11c3e5e moved etc/ to config/ and web/ to public/
This recipe adds doctrine migrations to the official Symfony recipes repository.