This could be quite a big patch and need a few re-rolls so the sooner we can get this in the better.

Meta Issue.
#1916134: Remove module_* deprecated functions

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

benjy’s picture

Issue tags: +Novice

Tagging

benjy’s picture

Status: Active » Needs review
FileSize
4.91 KB
alexpott’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/system.moduleundefined
@@ -3167,7 +3167,7 @@ function system_get_module_admin_tasks($module, $info) {
diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php

diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php
old mode 100644
new mode 100755

Careful with including unrelated change...

benjy’s picture

Status: Needs work » Needs review
FileSize
4.8 KB

Good spot, that was trying to re-install the other day. New patch attached.

Sam152’s picture

I'm brand new to core development but the above patch looks good to me. Not sure if it should be recreated to respect the 80 character comment limited as per:

// this can quickly lead to Drupal::moduleHandler()->implementsHook() being called several thousand times

But other than that, looks fine to me.

Sam152’s picture

Status: Needs review » Reviewed & tested by the community
claudiu.cristea’s picture

Status: Reviewed & tested by the community » Needs work

Only coding standards comments:

+++ b/core/lib/Drupal/Core/Extension/CachedModuleHandler.phpundefined
@@ -99,7 +99,7 @@ public function resetImplementations() {
+    // this can quickly lead to Drupal::moduleHandler()->implementsHook() being called several thousand times

Line too long, wrap to 80 characters. See https://drupal.org/node/1354.

+++ b/core/lib/Drupal/Core/Extension/CachedModuleHandler.phpundefined
@@ -139,7 +139,7 @@ protected function getImplementationInfo($hook) {
+        // Since Drupal::moduleHandler()->implementsHook() may needlessly try to load the include file again,

Same here.

benjy’s picture

Status: Needs work » Needs review
FileSize
5 KB

New patch attached.

benjy’s picture

Status: Needs review » Reviewed & tested by the community

Settings back to RTBC since it was just a comment change.

catch’s picture

+++ b/core/includes/install.incundefined
@@ -993,7 +993,7 @@ function drupal_requirements_severity(&$requirements) {
+  if (Drupal::moduleHandler()->implementsHook($module, 'requirements')) {
     // Check requirements

ModuleHandler::invoke() already calls implementsHook() and returns NULL if nothing's there, so we should remove it altogether.

+++ b/core/modules/search/search.moduleundefined
@@ -1062,9 +1062,9 @@ function search_box_form_submit($form, &$form_state) {
 function search_data($keys, $module, $conditions = NULL) {
-  if (module_hook($module, 'search_execute')) {
+  if (Drupal::moduleHandler()->implementsHook($module, 'search_execute')) {
     $results = module_invoke($module, 'search_execute', $keys, $conditions);
-    if (module_hook($module, 'search_page')) {
+    if (Drupal::moduleHandler()->implementsHook($module, 'search_page')) {
       return module_invoke($module, 'search_page', $results);

Again this is also checked internally by module_invoke().

benjy’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
6.54 KB

Thanks, I wasn't aware of that check. New patch attached.

catch’s picture

Status: Needs review » Needs work
benjy’s picture

any reason this still needs work?

catch’s picture

Status: Needs work » Needs review

Nope, I forgot to change status with #10, then crossposted with #11.

benjy’s picture

Status: Needs review » Reviewed & tested by the community

Setting back.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Commited/pushed to 8.x, thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.