ModuleManager load module childs dependentsLoad childs modules #50
Conversation
| $this->loadFinished = 0; | ||
| if ($this->loadFinished > 0 && $this->parentModule && $afterCurrent) { | ||
| $childModule = is_object($module) | ||
| ? array($moduleName=>$module) |
There was a problem hiding this comment.
This looks interesting, but I'm uncertain how it should be used. As such:
- Please rebase against current develop branch. In particular, how you test against events has changed to ensure you can test against both zend-servicemanager v2 and v3.
- Please provide usage documentation: how do you declare a child module? what happens when a child module has its own dependencies? and so on.
- What happens if a child module is also declared in the module list?
Essentially, some documentation needs to be done, as well as a number of code changes, before we can accept this.
| * | ||
| * @var array | ||
| */ | ||
| protected $childsModules = []; |
There was a problem hiding this comment.
Please use $childModules throughout ("childs" is not grammatically correct).
| $this->loadFinished = 0; | ||
| if ($this->loadFinished > 0 && $this->parentModule && $afterCurrent) { | ||
| $childModule = is_object($module) | ||
| ? [$moduleName=>$module] |
There was a problem hiding this comment.
Run this against phpcs; we require spaces around all operators.
| * @param ModuleEvent $e | ||
| * @return void | ||
| */ | ||
| public function onLoadChildsModules(ModuleEvent $e) |
There was a problem hiding this comment.
This should be renamed to onLoadChildModules.
| @@ -62,6 +62,7 @@ public function testDefaultListenerAggregateCanAttachItself() | |||
| 'Zend\ModuleManager\Listener\OnBootstrapListener', | |||
| 'Zend\ModuleManager\Listener\ConfigListener', | |||
| 'Zend\ModuleManager\Listener\LocatorRegistrationListener', | |||
| 'Zend\ModuleManager\ModuleManager', | |||
There was a problem hiding this comment.
Shouldn't this already be registered (due to registration of the onLoadModules listener)? Why is this change necessary?
| @@ -140,7 +170,7 @@ public function loadModules() | |||
| * @triggers loadModule | |||
| * @return mixed Module's Module class | |||
| */ | |||
| public function loadModule($module) | |||
| public function loadModule($module, $afterCurrent = false) | |||
There was a problem hiding this comment.
Document the new $afterCurrent argument, please. What are valid values?
| public function init($moduleManager) | ||
| { | ||
| $moduleManager->loadModule('LoadChildsModule2', 'after'); | ||
| $moduleManager->loadModule(['SomeModule' => new \SomeModule\Module()], 'after'); |
There was a problem hiding this comment.
Shouldn't the second argument be a boolean true? While non-empty strings evaluate as truthy, using them for boolean conditionals is brittle.
|
Closing due to inactivity and lack of response by author to feedback and questions raised. |
Allow load modules (from Module::init()) after current module.
Analog of zendframework/zendframework#5651, but:
load modules A, B - configs order is A, B.
load modules A -> B - configs order is A, B.
recreated from #47