Skip to content

Doctrine Migrations #57

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Doctrine Migrations #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10b78c4
Added DoctrineMigrationsBundle.
May 6, 2017
5a76bd2
Rename directory to match Packagist.
May 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions doctrine/doctrine-migrations-bundle/1.2/etc/packages/migrations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
doctrine_migrations:
dir_name: '%kernel.project_dir%/DoctrineMigrations'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why adding it at the project root? Shouldn't it rather be in src? Or maybe in etc?

Copy link
Member

Choose a reason for hiding this comment

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

this should not be at the root. Migrations are code, and so should be in src/.

Btw, this means that the default value of %kernel.root_dir%/DoctrineMigrations will work fine for people putting their kernel in src (which is the case in Flex).

Copy link
Member

Choose a reason for hiding this comment

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

The name of this option looks confusing. dir_name would be DoctrineMigrations ... but the value is an absolute path.

namespace: App\Migrations
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need both the namespace and the "dir_name"?

Copy link
Member

Choose a reason for hiding this comment

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

dir_name defines the path where migrations are looked for when loading them (and the place where they are generated). I agree with you that the name is unfortunate as it is a path.
namespace defines the namespace used when generating new migrations (you never create the boilerplate code of migrations by hand unless you are crazy, as the bundle has CLI commands for that).

And we need to define them explicitly here, because the recipe decided to use non-default values.

table_name: migration_versions
name: Application Migrations
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of this name option? The name is vague, the default value doesn't help me and it's not explained either in the official docs.

Copy link
Member

Choose a reason for hiding this comment

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

I would remove this config:

  • the table name you provide here is just repeating the default value, which is fine for most people. So just rely on the default value instead. Adding the config explicitly in projects makes sense only if people are expected to modify it
  • the name is almost useless. It is used only for display purposes (and as the bundle does not support having several migration configurations in parallel, using a meaningful name for the migration config does not bring much)

Copy link
Member

Choose a reason for hiding this comment

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

Then, if possible, let's remove the "name" config option. Thanks!

9 changes: 9 additions & 0 deletions doctrine/doctrine-migrations-bundle/1.2/manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"bundles": {
"Doctrine\\Bundle\\MigrationsBundle\\DoctrineMigrationsBundle": ["all"]
},
"copy-from-recipe": {
"etc/": "%ETC_DIR%/"
},
"aliases": ["doctrine-migrations", "migrations"]
}