samsonasik / SanSessionToolbar

:zap: Session Toolbar that can be applied into Zend/Laminas DeveloperTools
MIT License
38 stars 11 forks source link

Remove getModuleDependencies implementation #76

Closed weierophinney closed 5 years ago

weierophinney commented 5 years ago

We're working on the Laminas Project migration tooling, and a user came across something today that we think may need to be addressed here.

SanSessionToolbar\Module defines getModuleDependencies(), and has it return a list including ZendDeveloperTools. When users perform a migration to Laminas packages, the equivalent package (laminas/laminas-developer-tools) is installed and activated in their application, but any attempt to run the application fails due to SanSessionToolbar indicating it requires the ZendDeveloperTools module.

The main reason to declare module dependencies predated heavy usage of Composer, and was a simple mechanism to ensure required dependencies were installed. In most cases, however, it is better to leverage Composer for this purpose.

The other reason to declare module dependencies was to ensure that listeners or services were available when the module performed bootstrap tasks. These scenarios can be achieved without usage of getModuleDependencies() in other ways:

In looking that the SanSessionToolbar\Module class, I see no reason for declaring the getModuleDependencies() method at all. The class does not work with any services or event emitters from the ZendDeveloperTools module at all, and all points of contact with that module are done via configuration.

My recommendation is to remove the method, which will help provide greatest forwards-compatibility.

samsonasik commented 5 years ago

@weierophinney thank you, I created PR #77 for it

samsonasik commented 5 years ago

implemented at #77