-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] replace alias in factory services #17867
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
Conversation
Q | A |
---|---|
Bug fix? | yes |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #17864 |
License | MIT |
Doc PR |
@@ -122,4 +124,13 @@ private function updateArgumentReferences(array $arguments, $currentId, $newId) | |||
|
|||
return $arguments; | |||
} | |||
|
|||
private function updateFactoryServiceReference($factoryService, $currentId, $newId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why introducing a new method if it is as short and only used one time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand now, the 2.7 version has more code, so looks fine to me as it will ease merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, we can indeed inline this for the 2.3 fix. I have no hard feelings about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep it like this.
Actually, 2.7 will need both this code and the code added in the other PR (as 2.7 has both the new |
Thank you @xabbuh. |
…abbuh) This PR was merged into the 2.3 branch. Discussion ---------- [DependencyInjection] replace alias in factory services | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17864 | License | MIT | Doc PR | Commits ------- 56f8798 replace alias in factory services