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

CommentFileSizeAuthor
#20 2045919-20-replace-module_implements-calls.patch39.49 KBparavibe
#18 2045919-18-replace-module_implements-calls.patch40.16 KBparavibe
#16 2045919-16-replace-module_implements-calls.patch40.28 KBparavibe
#9 2045919-9-replace-module_implements-calls.patch41.04 KBparavibe
#8 2045919-8-replace-module_implements-calls.patch31.43 KBparavibe
#4 2045919-4-replace-module_implements-calls.patch31.44 KBparavibe
#2 2045919-2-replace-module_implements-calls.patch31.49 KBparavibe

Comments

benjy’s picture

benjy commented
Issue tags: +Novice

Tagging

paravibe’s picture

paravibe commented
Assigned: Unassigned » paravibe
Status: Active » Needs review
StatusFileSize
new 2045919-2-replace-module_implements-calls.patch31.49 KB

Done.
Please review and commit.

benjy’s picture

benjy commented
Status: Needs review » Needs work

There is no need for \Drupal to be absolute. We only do that in documentation.

+++ b/core/includes/bootstrap.incundefined
@@ -1579,7 +1579,7 @@ function watchdog($type, $message, array $variables = NULL, $severity = WATCHDOG
-    foreach (module_implements('watchdog') as $module) {
+    foreach (\Drupal::moduleHandler()->getImplementations('watchdog') as $module) {
       $function = $module . '_watchdog';
       $function($log_entry);
     }
paravibe’s picture

paravibe commented
Status: Needs work » Needs review
StatusFileSize
new 2045919-4-replace-module_implements-calls.patch31.44 KB

OK, thanks benjy.
Left it only for these two files:
core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php
core/lib/Drupal/Core/Entity/DatabaseStorageController.php

Status: Needs review » Needs work

The last submitted patch, 2045919-4-replace-module_implements-calls.patch, failed testing.

paravibe’s picture

paravibe commented

I continue to work on this.

benjy’s picture

benjy commented

drupalarv, seems I mislead you a bit on the absolute paths vs relative. They have to be absolute in files with a namespace (which I didn't realise). Which is what caused your test failures.

Here's the relevant link to the docs so you can see for yourself :) https://drupal.org/node/1353118

paravibe’s picture

paravibe commented
Status: Needs work » Needs review
StatusFileSize
new 2045919-8-replace-module_implements-calls.patch31.43 KB

Thanks benjy!
This patch should be good.

paravibe’s picture

paravibe commented
StatusFileSize
new 2045919-9-replace-module_implements-calls.patch41.04 KB

New patch.
Replace reference in comments and some other files.

Status: Needs review » Needs work
Issue tags: -Novice

The last submitted patch, 2045919-9-replace-module_implements-calls.patch, failed testing.

paravibe’s picture

paravibe commented
Status: Needs work » Needs review

#9: 2045919-9-replace-module_implements-calls.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2045919-9-replace-module_implements-calls.patch, failed testing.

paravibe’s picture

paravibe commented
Status: Needs work » Needs review

#9: 2045919-9-replace-module_implements-calls.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2045919-9-replace-module_implements-calls.patch, failed testing.

paravibe’s picture

paravibe commented
Status: Needs work » Needs review
Issue tags: +Novice
StatusFileSize
new 2045919-16-replace-module_implements-calls.patch40.28 KB

A new one. With last core changes.

Status: Needs review » Needs work

The last submitted patch, 2045919-16-replace-module_implements-calls.patch, failed testing.

paravibe’s picture

paravibe commented
Status: Needs work » Needs review
StatusFileSize
new 2045919-18-replace-module_implements-calls.patch40.16 KB

Just forget to write absolute \Drupal path in some files, sorry.

benjy’s picture

benjy commented
Status: Needs review » Needs work

This patch no longer applies. If we can get a re-roll we can try get this in ASAP.

paravibe’s picture

paravibe commented
Status: Needs work » Needs review
StatusFileSize
new 2045919-20-replace-module_implements-calls.patch39.49 KB

New one. Will be good if we could apply it ASAP.

benjy’s picture

benjy commented
Status: Needs review » Reviewed & tested by the community

OK Let's try get this in :)

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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

ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
commented
Issue summary: View changes
Status: Closed (fixed) » Active

The watchdog function in bootstrap.inc still calls function_exists('module_implements'). This needs to be removed or updated (possibly to class_exists('\Drupal')?)

ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
commented

See #325169-81: Move error/exception handler higher up in the bootstrap process for why this function_exists is there.

We should be able to just remove it, but it will need manual testing to confirm there isn't another function that watchdog is dependent on (possibly t).

ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
commented
Issue tags: -Novice +@deprecated
ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
commented
Status: Active » Closed (fixed)

watchdog() has been rewritten, so #24 no longer applies