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

Detach Migrator from the CLI #795

Closed
9 tasks done
lcharette opened this issue Sep 9, 2017 · 4 comments
Closed
9 tasks done

Detach Migrator from the CLI #795

lcharette opened this issue Sep 9, 2017 · 4 comments
Assignees
Labels
bakery Related to the Bakery feature core feature request Feature request
Milestone

Comments

@lcharette
Copy link
Member

lcharette commented Sep 9, 2017

Migrator shouldn't be tied to so much to the CLI and migrate command. Right now, there's no standard way to test, outside of the migrate command, if a migration has been run. For example :

if (!Capsule::schema()->hasColumn('migrations', 'id')) {
$this->io->error("Migrations doesn't appear to have been run! Make sure the database is properly migrated by using the `php bakery migrate` command.");
exit(1);
}
// Make sure the required mirgations have been run
foreach ($this->dependencies as $migration) {
if (!Migrations::where('migration', $migration)->exists()) {
$this->io->error("Migration `$migration` doesn't appear to have been run! Make sure all migrations are up to date by using the `php bakery migrate` command.");
exit(1);
}
}

Should also help with #778.

Progress

@lcharette lcharette added bakery Related to the Bakery feature core feature request Feature request labels Sep 9, 2017
@lcharette lcharette self-assigned this Sep 9, 2017
@lcharette
Copy link
Member Author

lcharette commented Nov 30, 2017

Some notes and references for later:

@alexweissman
Copy link
Member

alexweissman commented Dec 5, 2017

Mockery

@lcharette
Copy link
Member Author

lcharette commented Dec 11, 2017

Most work has been done in the feature-migrator branch regarding this issue.

One thing that could left to do, beside updating learn, would be to re-implement the sprinkle option for the migrate:rollback command (and probably reset and refresh too). First, that option for the base migrate command doesn't make sense. Why would you want to migrate up a sprinkle, but not the other? But for rolling back, it does makes sense if you want to delete a previously loaded sprinkle you don't want anymore. So yeah, stil looking into this (and #730).

(Sprinkle support for migration is the reason the migrator wasn't moved to a separate package btw)

Some other notes in case I forgot :

  • We can't go away from using namespaces with our migrations. As pointed in my previous comment, Laravel doesn't use them (and doesn't have to deal with namespace issue) because they store all their migration in a single path OR migrate a single path at the time. This is something we can't do with sprinkles, as two sprinkles can have dependent migrations between each other and two sprinkles can have the same class names for their migrations. This is still taken care of more accurately in the new migrator since we don't enforce the version part of the migration anymore (can be in the top level).
  • Tests can now use the new migrator service to call in migrations and setup a clean test, in memory, SQLite database. So this should help with Support "non-interactive mode" for migrations and data seeds #778. Seeding the database is still an issue thought...

Last of all, this update should be (squashed) merge into 4.2.0 version. There no big breaking change, but some stuff got deprecated. So, just in case...

For a list of other fix in this branch : Changelog

And yes, I did learn how Mockery works. It's great.

@lcharette
Copy link
Member Author

This is a good idea...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bakery Related to the Bakery feature core feature request Feature request
Projects
None yet
Development

No branches or pull requests

2 participants