[5.5] Autowire migrations

[abellion]

This PR makes use of the container to autowire the migrations instances.

[browner12]

is there any immediate benefit from this? or is it more for consistency?

[GrahamCampbell]

Probably should use the contact here instead.

[sisve]

We've had migrations that weren't resolved from the container for several years now, why do we need it now? When do you need to resolve services to modify your database schema?

[deleugpn]

I know it's almost irrelevant, but this brings a performance impact as the Reflector will be used to instantiated migration classes. The reason I bring this up is that if there is no upside at all for this, we can see a tiny downside.

Also, tests are failing.

[abellion]

For consistency yes, and because we use MondoDB as our primary data storage and for some reasons we need services from the container. I think the performance impact is so tiny it shouldn't be a problem. I'm going to fix the tests.

[sisve]

Let's talk about breaking changes. What are we talking about here? In which scenarios will things blow up? Can third party packages be compatible with the current Migrator class and these new changes at the same time?

[deleugpn]

I agree. Talk about performance is pointless if there is a visible improvement. I only brought it up because we cannot see the improvement here. It would be nice to know how you're setting up MongoDB with migrations and how this solve that.

@sisve I don't think there's any breaking changes here. If we assume that nobody is binding the migration classes into the container, this will auto-wire the class regularly.
If somebody is binding it to the container, the container will just resolve the bound one, which seems the intended behavior. The reason I think nobody is binding it is because the migrator was not resolving it anyway.

[abellion]

Actually our own use case is related to MongoDB services, but I can imagine it's far from being the only one. I just don't see what's the benefit of NOT having these class autowired ?

BC layer should not be a problem as the class were constructed without any arguments (ie. an exception would be thrown). Except for default parameters I guess, but the container is taking care of them.

[sisve]

@deleugpn @abellion You're changing a method signature (Migrator::__construct). How would anyone that has derived from the class and implemented their own constructor be compatible with current and next version of the Migrator if there are two different parent::__construct to call?

[abellion]

@taylorotwell @GrahamCampbell Can you explain why it has been closed ?

[browner12]

all we currently see in this PR is change for change's sake. can you explain your use case, and what the benefit of this would actually be?

[abellion]

We are using an internal tool do deal with MongoDB, that needs to be autowired.
Today we use the app() function to get the services, but it feels weird and inconsistant.

Actually our own use case is related to MongoDB services, but I can imagine it's far from being the only one. I just don't see what's the benefit of NOT having these class autowired ?