Now that watchdog() is deprecated we should remove all usage, and then in a followup issue, remove the function itself

CommentFileSizeAuthor
#36 drupal-watchdog-2271251-36.patch41 KBParisLiakos
#27 drupal-watchdog-2271251-27.patch41 KBundertext
#25 drupal-watchdog-2271251-25.patch40.98 KBundertext
#23 drupal-watchdog-2271251-23.patch40.78 KBParisLiakos
#22 drupal-watchdog-2271251-22.patch46.75 KBjeroent
#21 drupal-watchdog-2271251-21.patch46.63 KBParisLiakos
#20 drupal-watchdog-2271251-20.patch45.13 KBParisLiakos
#18 remove_usages_of-2271251-18.patch44.73 KBstevenpatz
#17 interdiff.txt1.51 KBParisLiakos
#17 drupal-watchdog-2271251-17.patch45.72 KBParisLiakos
#16 interdiff.txt4.35 KBParisLiakos
#16 drupal-watchdog-2271251-16.patch45.86 KBParisLiakos
#14 interdiff.txt1.84 KBParisLiakos
#14 drupal-watchdog-2271251-14.patch48.68 KBParisLiakos
#12 drupal-watchdog-2271251-11.patch46.84 KBParisLiakos
#10 drupal-watchdog-2271251-10-do-not-test.patch208.51 KBParisLiakos
#10 interdiff.txt4.49 KBParisLiakos
#8 interdiff.txt2.08 KBParisLiakos
#8 drupal-watchdog-2271251-8.patch210.41 KBParisLiakos
#6 interdiff.txt9.53 KBParisLiakos
#6 drupal-watchdog-2271251-6.patch210.08 KBParisLiakos
#4 drupal-watchdog-2271251-4.patch208.31 KBParisLiakos
#2 drupal-watchdog-2271251-2.patch208.42 KBParisLiakos
#1 watchdog-2271251-1.patch11.81 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
StatusFileSize
new watchdog-2271251-1.patch11.81 KB

Here's FormValidator. I just *really* wanted those damn watchdog hacks gone :)

ParisLiakos’s picture

ParisLiakos commented
StatusFileSize
new drupal-watchdog-2271251-2.patch208.42 KB

this is probably too big to be in a single issue...maybe we split it to 2/3 parts..but anyway, lets have it green first

Status: Needs review » Needs work

The last submitted patch, 2: drupal-watchdog-2271251-2.patch, failed testing.

ParisLiakos’s picture

ParisLiakos commented
Status: Needs work » Needs review
StatusFileSize
new drupal-watchdog-2271251-4.patch208.31 KB

reroll

Status: Needs review » Needs work

The last submitted patch, 4: drupal-watchdog-2271251-4.patch, failed testing.

ParisLiakos’s picture

ParisLiakos commented
Status: Needs work » Needs review
StatusFileSize
new drupal-watchdog-2271251-6.patch210.08 KB
new interdiff.txt9.53 KB

lets try again

Status: Needs review » Needs work

The last submitted patch, 6: drupal-watchdog-2271251-6.patch, failed testing.

ParisLiakos’s picture

ParisLiakos commented
Status: Needs work » Needs review
StatusFileSize
new drupal-watchdog-2271251-8.patch210.41 KB
new interdiff.txt2.08 KB

one more

Status: Needs review » Needs work

The last submitted patch, 8: drupal-watchdog-2271251-8.patch, failed testing.

ParisLiakos’s picture

ParisLiakos commented
StatusFileSize
new interdiff.txt4.49 KB
new drupal-watchdog-2271251-10-do-not-test.patch208.51 KB
ParisLiakos’s picture

ParisLiakos commented
Status: Needs work » Postponed

#1289536: Switch Watchdog to a PSR-3 logging framework was reverted

ParisLiakos’s picture

ParisLiakos commented
Title: Remove usages of watchdog() » Remove usages of watchdog() from non-form and non-controllers
Status: Postponed » Needs review
StatusFileSize
new drupal-watchdog-2271251-11.patch46.84 KB

Ok, i splitted this to 3 issues.
This one
One for procedural code: #2272467: Remove usages of watchdog() from procedural code
One for forms, controllers and plugins: #2272481: Remove usages of watchdog() from forms, plugins and controllers

Status: Needs review » Needs work

The last submitted patch, 12: drupal-watchdog-2271251-11.patch, failed testing.

ParisLiakos’s picture

ParisLiakos commented
Status: Needs work » Needs review
StatusFileSize
new drupal-watchdog-2271251-14.patch48.68 KB
new interdiff.txt1.84 KB

Status: Needs review » Needs work

The last submitted patch, 14: drupal-watchdog-2271251-14.patch, failed testing.

ParisLiakos’s picture

ParisLiakos commented
Status: Needs work » Needs review
StatusFileSize
new drupal-watchdog-2271251-16.patch45.86 KB
new interdiff.txt4.35 KB

we cant really inject a logger in the module_handler because during install/uninstall container gets rebuilt and module handler ends up to have an outdated instance injected

ParisLiakos’s picture

ParisLiakos commented
StatusFileSize
new drupal-watchdog-2271251-17.patch45.72 KB
new interdiff.txt1.51 KB

Some fixes..i missed what MigrateMessage actually did

stevenpatz’s picture

stevenpatz
he/him
commented
StatusFileSize
new remove_usages_of-2271251-18.patch44.73 KB

Status: Needs review » Needs work

The last submitted patch, 18: remove_usages_of-2271251-18.patch, failed testing.

ParisLiakos’s picture

ParisLiakos commented
Status: Needs work » Needs review
StatusFileSize
new drupal-watchdog-2271251-20.patch45.13 KB
ParisLiakos’s picture

ParisLiakos commented
StatusFileSize
new drupal-watchdog-2271251-21.patch46.63 KB

reroll

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
StatusFileSize
new drupal-watchdog-2271251-22.patch46.75 KB

Patch no longer applied so I created a reroll.

ParisLiakos’s picture

ParisLiakos commented
StatusFileSize
new drupal-watchdog-2271251-23.patch40.78 KB

reroll.

After this patch, no usage remains:

/d8$ grep -r 'watchdog(' core/
core/modules/dblog/dblog.module: * @see watchdog()
core/modules/dblog/src/Tests/DbLogTest.php:    $this->assertEqual($count + 1, db_query('SELECT COUNT(*) FROM {watchdog}')->fetchField(), format_string('dblog_watchdog() added an entry to the dblog :count', array(':count' => $count)));
core/modules/simpletest/src/UnitTestBase.php: * watchdog(), \Drupal::moduleHandler()->getImplementations(),
core/modules/system/src/Tests/Module/ModuleTestBase.php:   * Called in the same way of the expected original watchdog() execution.
core/includes/bootstrap.inc: * @see watchdog()
core/includes/bootstrap.inc: * This is a wrapper function for watchdog() which automatically decodes an
core/includes/bootstrap.inc: * @see watchdog()
core/includes/bootstrap.inc: *   general practice is to use the name of the module calling watchdog().
core/includes/bootstrap.inc: * @see hook_watchdog()
core/includes/bootstrap.inc:function watchdog($type, $message, array $variables = array(), $severity = WATCHDOG_NOTICE, $link = NULL) {
core/includes/common.inc: * @see watchdog()
core/lib/Drupal/Core/Logger/LoggerChannel.php:   * @todo Move watchdog constants here when watchdog() is removed.
core/tests/Drupal/Tests/Core/Logger/LoggerChannelFactoryTest.php:// @todo Remove once watchdog() is removed.
core/tests/Drupal/Tests/Core/Logger/LoggerChannelTest.php:// @todo Remove once watchdog() is removed.

RTBC anyone?

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Argh

undertext’s picture

undertext commented
Status: Needs work » Needs review
StatusFileSize
new drupal-watchdog-2271251-25.patch40.98 KB

Status: Needs review » Needs work

The last submitted patch, 25: drupal-watchdog-2271251-25.patch, failed testing.

undertext’s picture

undertext commented
Status: Needs work » Needs review
StatusFileSize
new drupal-watchdog-2271251-27.patch41 KB

Oh. randomName() is now randomMachineName().One more reroll.

m1r1k’s picture

m1r1k commented
Issue tags: +#ams2014contest

herom queued 27: drupal-watchdog-2271251-27.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 27: drupal-watchdog-2271251-27.patch, failed testing.

Status: Needs work » Needs review

ParisLiakos queued 27: drupal-watchdog-2271251-27.patch for re-testing.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs review » Needs work 
+++ b/core/core.services.yml
@@ -150,7 +150,7 @@ services:
+    arguments: ['@request_stack', '@string_translation', '@csrf_token', '@logger.channel.form']

I didn't know we could do this :)

  1. +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.php
@@ -901,7 +901,7 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
-        watchdog('system', '%module module installed.', array('%module' => $module), WATCHDOG_INFO);
+        \Drupal::logger('system')->info('%module module installed.', array('%module' => $module));

@@ -1028,7 +1028,7 @@ public function uninstall(array $module_list, $uninstall_dependents = TRUE) {
-      watchdog('system', '%module module uninstalled.', array('%module' => $module), WATCHDOG_INFO);
+      \Drupal::logger('system')->info('%module module uninstalled.', array('%module' => $module));

    Should it be injected? 

  2. +++ b/core/modules/locale/src/PoDatabaseWriter.php
@@ -238,7 +238,7 @@ private function importString(PoItem $item) {
-        watchdog('locale', 'Import of string "%string" was skipped because of disallowed or malformed HTML.', array('%string' => $translation), WATCHDOG_ERROR);

+++ b/core/modules/migrate/src/MigrateMessage.php
@@ -26,7 +26,8 @@ class MigrateMessage implements MigrateMessageInterface {
+    $type = isset($this->map[$type]) ? $this->map[$type] : WATCHDOG_NOTICE;
+    \Drupal::logger('migrate')->log($type, $message);

    Should it be injected? 

  3. +++ b/core/modules/system/src/Access/CronAccessCheck.php
@@ -25,11 +25,11 @@ class CronAccessCheck implements AccessInterface {
+      \Drupal::logger('cron')->notice('Cron could not run because an invalid key was used.');
...
+      \Drupal::logger('cron')->notice('Cron could not run because the site is in maintenance mode.');

    Should it be injected? 

  4. +++ b/core/modules/views/tests/modules/views_test_data/src/Plugin/views/display/DisplayTest.php
@@ -96,7 +96,7 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
+    \Drupal::logger('views')->notice($form_state->getValue('test_option'));

    Same.

ParisLiakos’s picture

ParisLiakos commented

1. See #16
2. Those are being instantiated from all over the place, they are not in the container or controllers/plugins
3. It already uses \Drupal for the state service, so i didnt bother injecting it
4. Its a test so i didnt bother

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll, -#ams2014contest

Then let's RTBC it.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

No, it does not apply again :(

ParisLiakos’s picture

ParisLiakos commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
StatusFileSize
new drupal-watchdog-2271251-36.patch41 KB

thanks! i edited the patch manually to make it apply again.
conflicted with #2322889: Various setUp() and tearDown() methods are not protected

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 9be30ef and pushed to 8.0.x. Thanks!

  • alexpott committed 9be30ef on 8.0.x 
    Issue #2271251 by ParisLiakos, undertext, JeroenT, StevenPatz, tim....

Status: Fixed » Closed (fixed)

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