now that [#325665 has landed, it would be nice to get rid of the remaining places where we use:

foreach (module_list() as $module)) {
  // do something with a hook on each module

and replace them with:

foreach (module_implements($hook) as $module)) {
  // do something with $hook on each module

this will help to with #332003: cut the fat out of _drupal_bootstrap_full.

CommentFileSizeAuthor
#13 drupal-module_implements-334030-13.patch5.61 KBhefox
#11 drupal-module_implements-334030-10.patch5.61 KBhefox
#1 334030-module-list-used-hook.patch7.56 KBdamien tournoud

Comments

damien tournoud’s picture

damien tournoud commented
Status: Active » Needs review
StatusFileSize
new 334030-module-list-used-hook.patch7.56 KB
catch’s picture

catch
he/him
Primary language English
commented

Visually this looks great. I'm running all tests but it's taking a very long time, no fails so far :)

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

yay, more kittens saved. simple patch, looks good, all tests pass, RTBC.

catch’s picture

catch
he/him
Primary language English
commented

all tests passed for me too in the end.

We should document this as a best practice in the upgrade docs once it gets in.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

catch’s picture

catch
he/him
Primary language English
commented
Status: Fixed » Needs work

Marking to needs work for docs.

Anonymous’s picture

Anonymous (not verified) commented

@catch: where is it that the doc updates should go exactly?

catch’s picture

catch
he/him
Primary language English
commented

http://drupal.org/update/modules ought to do fine.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Closed (fixed)

closing, as i've put some docs in.

hefox’s picture

hefox commented
Version: 7.x-dev » 6.x-dev
Assigned: » Unassigned
Status: Closed (fixed) » Needs review
Issue tags: +Performance

As far as I can tell, its not an api change to backport this change to d6, and instead a performance improvement that should have no negative effects, so re-opening (and marking #1861562: Use module_implements('user') instead of module_list()/function_exists (for performance) as duplicate which filled before finding this).

did not backport the variable name only changes, e.g. node_invoke_nodeapi change.

Sorta related, #1861604: Skip module_invoke/module_hook in calling hook_watchdog (excessive function_exist) is for watchdog as it combines module_implements with module_invoke which is extra function_exists calls.

And spawned #1864188: user_filters invokes hook_permission with argument 'permission' ($function('permission')) after noticing the issue when backporting.

hefox’s picture

hefox commented
StatusFileSize
new drupal-module_implements-334030-10.patch5.61 KB

... I guess I could attach the patch

Status: Needs review » Needs work

The last submitted patch, drupal-module_implements-334030-10.patch, failed testing.

hefox’s picture

hefox commented
Status: Needs work » Needs review
StatusFileSize
new drupal-module_implements-334030-13.patch5.61 KB

modul_implements => module_implements

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.