This patch cleans up the entire module. It removes redundant code and while doing that also fixes a bug (a separate issue would have been unhandy). It prevents Windows users from seeing any setting at all (since they have no choice anyway) and Unix/Linux users from seeing the Windows-specific setting as well. The settings form has been moved to admin/settings/logging and the description has been simplified by moving part of it to the help page. Tests have been updated as well. Also, all occurences of the name 'Drupal' have been removed.
The form after the patch:
The help text before and after the patch:
Comment | File | Size | Author |
---|---|---|---|
#14 | syslog_cleanup_07.patch | 8.31 KB | Xano |
#11 | syslog_cleanup_06.patch | 8.07 KB | Xano |
#11 | Picture 48.png | 18.62 KB | Xano |
#8 | syslog_cleanup_05.patch | 8.58 KB | Xano |
#4 | syslog_cleanup_04.patch | 8.11 KB | Xano |
Comments
Comment #2
XanoI forgot to mention that. The patch was made outside of the Drupal root because of lack of CVS access. You will need to apply it using p1, IIRC.
Comment #3
XanoNeed to add OS X to the paragraph about UNIX/Linux.
Comment #4
XanoFollow-up patch. OS X is now mentioned as well.
Comment #5
keith.smith CreditAttribution: keith.smith commentedIMO, if the individual operating system sections are to be separated out like this, they should not be as h2 level headings (where they will conflict with the help topic), but should probably be list items with a bolded section at the front describing the OS.
Comment #6
XanoThe headings are wrong in Garland, since the help topic should be a <h1>.
Comment #8
XanoThis patch fixes a bug in system_watchdog() that caused a PHP error if the hook was invoked for the first time.
I suggest we change this as well:
The strings we have now don't add anything to the codes themselves.
Comment #9
Dries CreditAttribution: Dries commentedThis looks good to me but I don't have a MS Windows environment to test this on.
Comment #11
XanoComment #12
brianV CreditAttribution: brianV commentedThe Linux portions look good to me. However, I can't test the Windows portions.
Comment #14
XanoTagging this WTF because Windows users should *not* be able to select Linux facilities.
Comment #15
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!
Comment #16
cburschkaI'd open a separate issue, but this is titled "cleanup" so it fits.
The help text contains a link to http://www.rt.com/man/syslog.5.html, which is a 404. This should be replaced with a more durable man link.
Comment #17
XanoWhat about http://www.linuxmanpages.com/man2/syslog.2.php?
Comment #18
moshe weitzman CreditAttribution: moshe weitzman commentedpls open new issue.
Comment #20
Xano