file_munge_filename() doc mentions watchdog but shouldn't
allain - October 2, 2009 - 20:05
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | documentation |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
| Issue tags: | Novice |
Description
Documentation refers to watchdog in reference to its alerts parameter, but it's not called at all and the only other method it calls, drupal_set_message, doesn't call it either.
Maybe it should be removed?
#1
Agreed. watchdog() is not being called, and should be removed from the doc.
Please fix in Drupal 7 first, then backport patch to Drupal 6.
#2
Here's some better (I think) doc for file_munge_filename().
#3
Yes, patch applies on head.
Comment changes only. Seems thorough.
Watchdog was removed. Ship it!
Yeah! First patch review!
#4
Overall, a pass from me. Applies fine, solves the original problem by removing reference to watchdog(), and clarifies the rest of the documentation too. I might clarify the line which reads
As it isn't immediately clear what this variable is, where it is set, etc.; but I would still pass the patch even if this line was kept as it currently is.
#5
Changing status to RTBC - accidentally changed away from this as we were both reviewing at the same time.
But FWIW, shouldn't patches only be set to RTBC when they've been received two positive reviews?
#6
Committed to CVS HEAD. Thanks!
#7
Needs port to Drupal 6.
#8
Here's a patch for Drupal 6.