-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fix for zf generate-migration command #21
base: master
Are you sure you want to change the base?
Conversation
…snamed variable and misplaced _initDoctrineResource() call
Hi Ben, zf generate-migration doctrine class-name d-from-database m-from-models Apparently "d-from-database" and "m-from-models" are actually just perhaps naming suggestions for helping developers to visually determine the purpose of a particular migration just by looking at the migration filename? Is that correct? If so while I understand your goal, I think it would be far less confusing if you changed the signature to simply this, leaving the matter of naming preference to the developer (perhaps mention the naming recommendation in the documentation): zf generate-migration doctrine migration-name |
You should probably run it like this To create a migration class from database. It would not be empty. I will run a test to verify. |
Yes it works fine, the class (with the database calls) is created nicely. |
I noticed some weird behavior with the zf generate-migration command: I wanted to create a migration set so I used --m-from-database to get a diff to my existing schema.yml. However the up/down methods wanted to drop/create the whole tables instead of addColumen/removeColumn. I traced thru the generateMigration code, which calls Doctrine_Core::generateMigrationsFromDiff(..) , my yml against models created from my db, however Doctrine/Migration/Diff.php::_generateModels seems to fail to create the temp files and therefore the migration diffs against an empty 'from' schema. |
I think this is unavoidable, because the way that we name the classes vs tables. Going straight from the database you have no way of mapping tables to classnames. Example: schema.yml Here we can map to table name. Database No way to map back to the classname. So if I understand this correctly the only way of producing a correct diff is to do as you did, that is to keep two versions of the yaml file. I'm trying the same approach actually because writing the migration classes manually seems like too much work. If anyone has any ideas as to how you can map from database tables to classname I'd like to hear it. Edit: Would it be able to map back to classname if the table is named accordingly? Like table: name_model_test |
no you got me wrong. _generateModels usually works. My tables are named like the classes (name_model_test). Therefore I CAN generate models from the database. But when doing generate-migration --d-from-database _generateModels fails to create the temporary model files! I think its a problem with the paths to them being messed up. But didnt have the time to trace on so I added ad quickfix that works on 2 yaml files.. |
Oh. Perhaps I was a bit too fast to apply my own findings to your problems. My temporarye files are created alright, so it should be possible to get it to work with your setup. Anyway I don't have time to work on it right now, but maybe later down the road, if noone else steps up to do it. |
so for you, when you do a --d-from-database it produces a correct diff with addColumn etc of the changes and not a whole dropTable/addTable ? (btw I am encountering that bug on a Mac) |
I will have to check.. But it will be a few days until I get round to i. |
Hi Ben,
I've been having great success using zf-doctrine, however it definitely seems as if the migration feature is broken. A glance at DoctrineProvider.php indicates that this was due to a misnamed variable ($migrationsPath instead of $migratePath) and a misplaced call to $this->_initDoctrineResource(). I've fixed these issues within this doctrineproviderfix branch, and a migration is now being created (and the errors have disappeared).
Unfortunately it is unclear what exactly this migration file (which is successfully generated within the migrations directory is supposed to look like. A look at a generated migration file (generated with my patch in place) shows empty up() and empty() down methods which clearly seems incorrect. Nonetheless, the two changes found in this patch strike me as a step in the right direction towards fixing this problem.
Could you confirm and let me know? Happy to keep helping you out with this project if you'd like me to.
Jason