Meta issue: #2205673: [meta] Remove all @deprecated functions marked "remove before 8.0"

Now that this function has 0 usage we can remove it

CommentFileSizeAuthor
#12 drupal-watchdog-2324791-12.patch8.62 KBParisLiakos
#8 drupal-watchdog-2267545.patch9.23 KBParisLiakos
#6 Drupal-remove_watchdog-2324791-5.patch4.79 KBMichael Hodge Jr
#6 interdiff-2324791-1-5.txt2.13 KBMichael Hodge Jr
#2 Drupal-remove_watchdog-2324791-1.patch6.92 KBMichael Hodge Jr

Comments

Michael Hodge Jr’s picture

Michael Hodge Jr commented
Assigned: Unassigned » Michael Hodge Jr

I'm going to try to take a stab at this one.

Michael Hodge Jr’s picture

Michael Hodge Jr commented
Assigned: Michael Hodge Jr » Unassigned
Status: Active » Needs review
StatusFileSize
new Drupal-remove_watchdog-2324791-1.patch6.92 KB

Here is my first attempt. There were a few instances in the code where watchdog() was referred to in the comments, but I didn't change those as I wasn't sure if I should then reference \Drupal::logger or how that should be worded. An example is in core/includes/bootstrap.inc around line 785. 

/**
 * Logs an exception.
 *
 * This is a wrapper function for watchdog() which automatically decodes an
 * exception.
 */

Status: Needs review » Needs work

The last submitted patch, 2: Drupal-remove_watchdog-2324791-1.patch, failed testing.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

I think we should keep these references in comments but instead of watchdog(), something like \Drupal::logger($channel)->log($severity, $message, $context) ?

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Issue summary: View changes
Michael Hodge Jr’s picture

Michael Hodge Jr commented
StatusFileSize
new interdiff-2324791-1-5.txt2.13 KB
new Drupal-remove_watchdog-2324791-5.patch4.79 KB

I think I removed a little too much in the first patch, which is why the testing failed. I'm undoing my changes to the test factories.

Michael Hodge Jr’s picture

Michael Hodge Jr commented
Status: Needs work » Needs review
ParisLiakos’s picture

ParisLiakos commented
StatusFileSize
new drupal-watchdog-2267545.patch9.23 KB

hmm, i forgot to upload the patch..i think its more complete than the already posted ones

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/simpletest/src/UnitTestBase.php
@@ -15,7 +15,7 @@
- * watchdog(), \Drupal::moduleHandler()->getImplementations(),
+ * \Drupal::moduleHandler()->getImplementations(),
  * \Drupal::moduleHandler()->invokeAll() etc.

well, should we not mention calling a logger? i think it doesn't matter though because that UnitTestBase will be removed at some point anyway. For phpunit tests its just clear that there is no database available.

Status: Needs review » Needs work

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

Status: Needs work » Needs review

herom queued 8: drupal-watchdog-2267545.patch for re-testing.

ParisLiakos’s picture

ParisLiakos commented
StatusFileSize
new drupal-watchdog-2324791-12.patch8.62 KB

#2324789: Remove UnitTestBase is in :D so that chunk is now vanished

greping for 'watchdog(' returns 0 results

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Amsterdam2014

The patch looks good to me.

alexpott’s picture

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

What is the plan for watchdog_exception()? Also the CR https://www.drupal.org/node/2270941 needs updating.

Committed ebdca18 and pushed to 8.0.x. Thanks!

  • alexpott committed ebdca18 on 8.0.x 
    Issue #2324791 by Michael Hodge Jr, ParisLiakos: Remove watchdog().
ParisLiakos’s picture

ParisLiakos commented

What is the plan for watchdog_exception()?

after some thinking about it, i opened #2351099: watchdog_exception() hides the exception from logger implementations

Status: Fixed » Closed (fixed)

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