-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Replace custom service names with the class names #20013
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
Replace custom service names with the class names #20013
Conversation
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
kamil-tekiela
left a comment
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.
And with this change it makes the loader leaner and faster:
return static function (ContainerConfigurator $configurator): void {
$services = $configurator->services();
/** @param array<'services', array<string, array{class: string, arguments?: array<string>, factory?: callable}>> $servicesFile */
$loadServices = static function (array $servicesFile, ServicesConfigurator $services): void {
foreach ($servicesFile['services'] as $serviceName => $service) {
$theService = $services->set($serviceName, $service['class']);
if (isset($service['arguments'])) {
foreach ($service['arguments'] as &$argumentName) {
if ($argumentName[0] !== '@') {
continue;
}
$argumentName = new Reference(substr($argumentName, 1));
}
$theService->args($service['arguments']);
}
if (! isset($service['factory'])) {
continue;
}
$theService->factory($service['factory']);
}
};
$servicesFile = include ROOT_PATH . 'app/services.php';
$loadServices($servicesFile, $services);
$servicesFile = include ROOT_PATH . 'app/services_controllers.php';
$loadServices($servicesFile, $services);
};
As only other services are used as arguments, there's no need for it. Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #20013 +/- ##
============================================
+ Coverage 62.05% 62.14% +0.08%
+ Complexity 16112 16099 -13
============================================
Files 676 676
Lines 59990 59981 -9
============================================
+ Hits 37227 37274 +47
+ Misses 22763 22707 -56
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
| if (is_string($service)) { | ||
| $services->alias($serviceName, $service); | ||
| continue; | ||
| } |
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.
Is there a reason why this remains? Didn't you remove all aliases in this PR?
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.
This can be used for example to register an an interface as a service and make it an alias to a concrete implementation. I want to experiment with that to see if makes sense for phpMyAdmin. If it does not I'll remove that part.
Thinking about it now. It would have been better to remove that and add it back later if needed, because now it's just dead code.
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.
Addressed by #20015.
No description provided.