-
-
Notifications
You must be signed in to change notification settings - Fork 215
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
Add service migrations (POC) #524
base: 3.4.x
Are you sure you want to change the base?
Conversation
ea5caf6
to
bfa81e2
Compare
Here you can find a similar but much simpler implementation. My solution works without extending the autoloader. |
This PR will require to register migrations as services, right? If we go down that road, why would I need them to be service subscribers? I mean, I could just inject all dependencies directly, couldn't I? |
@derrabus The reason for my approach are the standard migration arguments: |
bfa81e2
to
03b1a62
Compare
@derrabus I've updated the implementation to use bound arguments instead of service locators. |
@greg0ire any plan when this will be merged? |
No |
@greg0ire what is blocking this from beeing merged? |
For starters:
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
03b1a62
to
28d9f01
Compare
28d9f01
to
d2f8cb8
Compare
d2f8cb8
to
cf481fc
Compare
I know it's just a POC for now, but since I'm asked what's blocking, I'd add that docs are probably necessary for this sort of thing. |
I started testing this PR with real world code. You should work on making this unnecessary:
|
Inspired by #521 (comment)
This is a proposed implementation for registering migrations as services.
To get this working Symfony needs to be able to autoload migrations:
Then, import the migrations as services:
Of course, an alternative approach would be to move the
migrations
folder tosrc
and change the namespace toApp\Migrations
.Once that is done, all you need to do is override the constructor and inject the desired services.
I haven't added any tests yet, as I was hoping to get some feedback on my approach first.