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

Us of different migrations dirs #40

Closed
tvbeek opened this issue Dec 11, 2012 · 19 comments
Closed

Us of different migrations dirs #40

tvbeek opened this issue Dec 11, 2012 · 19 comments

Comments

@tvbeek
Copy link

tvbeek commented Dec 11, 2012

Is there a possibility to add multiple directory's to read the migrations?
I want this to set the migrations in the module (a git submodule)
but run the migrations from the base project.

@ruckus
Copy link
Owner

ruckus commented Dec 11, 2012

Currently this is not available. It does raise the question: during the generation process where it write the file to? The first entry in the array of directories?

@tvbeek
Copy link
Author

tvbeek commented Dec 11, 2012

Maybe an optional option in the config with an array of path to read from (and use the existing option to read from and write the files during the generation process)

@timtonk
Copy link
Contributor

timtonk commented Jan 28, 2013

Is that "optional option" must belongs to each database or you want just array declaration for 'migrations_dir' option?

@tvbeek
Copy link
Author

tvbeek commented Jan 28, 2013

I prefer an array declaration for the migrations_dir option.

@timtonk
Copy link
Contributor

timtonk commented Jan 28, 2013

Check it. What if it will be a key-array like this one: array ("main" => %ordinary_path%, "foo" => %another_path% ... )?
And for convenience and reverse compatibility this option still should accept ordinary string with path without any array declaration.
Is it suitable?

@timtonk
Copy link
Contributor

timtonk commented Jan 28, 2013

Oh, forget second part. And you could generate migrations in selected dir by corresponding param in CLI like, for instance, 'path=foo'.

@tvbeek
Copy link
Author

tvbeek commented Jan 28, 2013

That key array sounds like a good solution.

@salimane
Copy link
Collaborator

After reading the comment I still could not get what the end result of this would be. Some real command use cases with examples could be appreciated. What are the keys of the array ?
Thanks

@timtonk
Copy link
Contributor

timtonk commented Jan 28, 2013

No problem.
Keys of the array - just some name for specify pathes. Now I'm use it in error message by migration similarity.
User can specify 'migrations_dir' as well as ordinary string (default state).

Known use-cases:

  1. Creating migration
    1.1 User with default state - just will get processing for the only folder. If user sets 'dir' param - just pass it without error.
    1.2 User with array in 'migrations_dir':
    1.2.1 User specified 'dir' param at script running - I use this param as destination, but will check all folders for unique migration name. If there is no path in 'migrations_dir' with such param as name - config error
    1.2.2 User doesn't specified 'dir' param at script running - I use the first key from 'migrations_dir' as destination, but will check all folders for unique migration name. If there is no path in 'migrations_dir' with such param as name - config error
  2. Migrate - will form common list of tasks from all dirs from 'migrations_dir' and migrate by this list.

Feel free to correct me if i made mistake.

Real command at this moment: php ruckus.php db:create [env=production] [dir=foo]
db:migrate command without changes.

@timtonk
Copy link
Contributor

timtonk commented Jan 28, 2013

Ok guys, i did it. But there is some points.

  1. Review my code as critical as you can
  2. I. Just. Can't. Create. Any. Test. It's my weakest part.

Will answer any questions through 12 hours.

@salimane
Copy link
Collaborator

The thing I don't understand is why do you want multiple directories for migrations. A migration directory is related to a database. if you have multiple databases, then you need multiple environment. As of now migrations are structured based on the database. So different databases migrations are placed in different directories named according to the database name.

With this change the migrations of a particular database would be scattered over multiples folders. Does rake works like this ? It's like this will add complexity with no obvious benefit.

Thanks

@timtonk
Copy link
Contributor

timtonk commented Jan 28, 2013

This question need to address to tvbeek. I'm just implement it.
Аlternatively - you will decline my pull request and tvbeek could use my branch for this feature.
BTW, agree with you about "complexity with no obvious benefit".

@tvbeek
Copy link
Author

tvbeek commented Jan 29, 2013

When you use modules in a project you want to have the database changes in a module and not in the project. Because you can reuse the module and don't want to need to copy the migration files.

Like a simple project structure

/www
/modules
    /base
        /migations
    /auth
        /migrations
    /cms
        /migrations
/ruckusing-migrations

You have one directory with ruckusing-migrations. And multiple directories with migrations for the same database.

@salimane
Copy link
Collaborator

@tvbeek I think I now understand a little bit your problem. I think a good solution would have to be carefully thought so as to not break so many things. @tonkonogov pr is a good start but I'm pretty sure it's breaking something somewhere still not tested. I will think about it carefully and push some codes around the end of this week basing off #76 . And this will badly need some good test cases.
Thanks

@timtonk
Copy link
Contributor

timtonk commented Jan 29, 2013

I tested locally migrating and generating migrations with old config file as well as config file with 2 dirs. Still doesn't tested any other tasks.
I'll try to create unit tests for this feature at nearest weekend.

@salimane
Copy link
Collaborator

salimane commented Feb 1, 2013

@tvbeek please checkout the new branch module-migration-dir to see if it solves your problem without any error.

```git clone -b module-migration-dir https://github.com/ruckus/ruckusing-migrations.git ruckusing-migrations-test/`

@salimane
Copy link
Collaborator

salimane commented Feb 1, 2013

I forgot to mention the new command:

php ruckus.php db:generate create_users_table module=cms
php ruckus.php db:migrate

of course your migration_dir would be like 'migration_dir' => array('default' => '/default/path/migrations', 'cms' => '/path/to/cms/migrations')

I'm working on an update for the test cases that will gives us more test coverage. So if @tonkonogov sends a pr for the unit tests, I will just merge them. If not, when I'm done with that "tests-branch", it will cover this change

@timtonk
Copy link
Contributor

timtonk commented Jun 16, 2013

After 5 months of bad things I have finally returned to my promise about tests. I didn't forget %)

@salimane
Copy link
Collaborator

@tonkonogov I've merged your changes. Thanks a lot

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

No branches or pull requests

4 participants