This is a follow-up from #1331486: Move module_invoke_*() and friends to an Extensions class, in particular some of Crell's comments in #1331486-45: Move module_invoke_*() and friends to an Extensions class:

Is this a direct port of existing code? Because it looks like it replicates existing design flaws, too. Folding multiple operations (load bootstrap module list and non-bootstrap list) into the same operation was never a good idea, so we shouldn't keep doing it if we don't have to.

This was regarding the loadAll() method. Also, moduleList() and systemList() take a $type parameter for bootstrap v. non-bootstrap - these operations need to be separated out.

We really ought to pull themes out of this, too. Ideally they should go in their own separate object.

Comments

catch’s picture

catch
he/him
Primary language English
commented

I think we can probably get away with removing all bootstrap hooks, so there's only ever one module list. If this is the main cause of the complexity could we try to do that here?

Crell’s picture

Crell commented
Component: wscci » base system
Issue tags: +WSCCI

I think we should eliminate bootstrap hooks one by one. Each hook_boot implementation has slightly different needs. But we can use this as a meta issue. I've not made a list of all of them yet, but ban module's getting addressed here: #1794754: Convert ban_boot() to an event listener

But yes, eliminating hook_boot() and hook_init() in favor of high priority request listeners would make the dual module list problem go away.

Crell’s picture

Crell commented

Is this still relevant, or does it open up again after #1331486: Move module_invoke_*() and friends to an Extensions class lands? It's RTBC as I write this...

katbailey’s picture

katbailey commented
Status: Postponed » Closed (duplicate)

Oh, this was from way back when I was taking more of a copy&paste approach to the patch :-P All that has been taken care of in the current patch.