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

Get rid of doctrine-orm-module #2

Closed
codeliner opened this issue May 5, 2015 · 15 comments
Closed

Get rid of doctrine-orm-module #2

codeliner opened this issue May 5, 2015 · 15 comments

Comments

@codeliner
Copy link

@gianarb Sorry, I was not able to test it today. I need to resolve some dependency issues first. I hope I find the time tomorrow.
But what I've already seen is that the repo continues to depend on doctrine-orm-module. So actually it won't add much benefit. Is it possible to reduce dependency to the DoctrineModule only?

@gianarb
Copy link
Owner

gianarb commented May 5, 2015

Hi! In this step I have moved a lot of code from DoctrineOrmModule.. In the second time I will try to remove DoctrineOrm.. :)

Now doctrineormmodule work without migrations deps

Sorry but it's an hard work eheh

@codeliner
Copy link
Author

@gianarb Hi, do you have a time schedule for the next step? Does it make sense if I start working on it? I won't have so much time this month (I move to a new city) but maybe I find some time to have a look.

@gianarb
Copy link
Owner

gianarb commented May 7, 2015

In this moment I have not idea.. :) I'm studying a good method to do this process.. have you proposals?

@codeliner
Copy link
Author

I need to have look at the source first. What exactly are the problems in the process? Hard dependencies between migrations and doctrineormmodule?
Without knowing it better I would say migrations depend on doctrine/dbal and CLI integration. The DoctrineModule should provide the needed infrastructure/ZF2 integration for both, right?

@gianarb
Copy link
Owner

gianarb commented May 8, 2015

Yes, if you try to search ORM in this repository you will see that there are few references between this two module..

In this moment I have not any project that use migrations..
I have need a lot of time to init the tests :)

this is the truth 👷

@codeliner
Copy link
Author

:-) ok. I have two apps running in production which both use ORM + migrations and two open source apps which use doctrine/dbal + migrations. So I have some apps available to perform tests.
I will have a look at the sources to get an overview and maybe also look at the doctrine* bundles where the problem seems to be solved already. As soon as I have an idea I will write it here to discuss further steps.

@gianarb
Copy link
Owner

gianarb commented May 8, 2015

eheh thanks! Would you share with me your two open source project that use only migrations? :)
Maybe I can try to use them with my starting point!

@codeliner
Copy link
Author

sure.
One is a sample application for my CQRS + ES ZF2 module:
https://github.com/prooph/proophessor-do

Using the second one is not so easy because it is under heavy development so I suggest you start with the first one and if it works I can try to include it in my second app.

@gianarb
Copy link
Owner

gianarb commented May 8, 2015

Perfect.. I wait your feedback! :)
Thanks a lot!

@codeliner
Copy link
Author

I had a look at the sources ...
As fare as I can see we have two dependencies between this module and doctrineormmodule:

  1. DoctrineORMModule\Service\DBAL*Factory
    • IMHO these factories should be located in DoctrineModule, they are related to doctrine/dbal, the ORM only depends on them
  2. DoctrineORMModule::initializeConsole
    • You've already moved command initialization for migration commands to the migration module but the DoctrineORMModule injects some helpers which are used by the migration commands. A blocker is the EntityManagerHelper. This cannot be moved to the DoctrineModule! doctrine/migrations depends on doctrine/orm hence DoctrineMigrationsModule depends on DoctrineORMModule :-(

Currently I see no chance of getting rid of the dependency. IMHO migrations must continue to be part of DoctrineORMModule beause they are tightly coupled through doctrine.

@gianarb
Copy link
Owner

gianarb commented May 9, 2015

Thanks for this feedback.. In my opinion we can split DoctrineORM not for your case but for another implementation.

  • we will use doctrineORM and not migrations..
  • doctrine migrations is another project..

What do you think?

@codeliner
Copy link
Author

From your POV it makes sense. Not every project needs the migration functionality, but for the MigrationsModule itself it makes no sense. In theory yes. I mean I like the idea, but I think these hard dependencies listed above make it impossible to split migrations in a clean way. Look at the current source of your module. What do you have? Two factories and a module listener which injects the migrations commands. The CLI is provided by DoctrineModule, the connection by DoctrineORMModule, the Driver by DoctrineModule, the EntityManager by DoctrineORMModule, and so on. I think you get the point. The logic or pre conditions are spread over two modules and DoctrineMigrationsModule would depend on both. Most of the maintenace work has to be done in the other modules anyway, so what is the real advantage of splitting the migrations? The existence of the migrations package as part of the ORM has no impact. If you need it, it is their and if not, you don't have to use it.

@gianarb
Copy link
Owner

gianarb commented May 20, 2015

Hi @codeliner thanks for this issue!! :) It's very important for me your feedback! :)
I have spoken about other devs and with doctrine-dev-team about this idea and now we decided..

DoctrineORMModule will maintain doctrine\migrations
https://twitter.com/GianArb/status/601072388982362112

:) Thanks

PS sorry I don't know your twitter name

@gianarb gianarb closed this as completed May 20, 2015
@codeliner
Copy link
Author

@gianarb thanks for the info. good decision 👍

PS you can't know my twitter name, because I have no account on twitter :-). Maybe it is time to create one ...

@gianarb
Copy link
Owner

gianarb commented May 20, 2015

Very hard search ihih

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

2 participants