Clean up API docs for syslog

Huh. Will that actually work in the API module as documentation for that DEFINE? I kind of doubt it. We should test that before committing this patch. Probably the docblock would need to be inside the if() statement... I can test this later.

Also, when linking to function docs on php.net, we try to use a non-language URL (/en/ should not be in the URL). Check other examples in Drupal core.

Amazing that the syslog module wouldn't need any other fixes!

Um... I am not sure what you're asking? I'm just saying that the doc block for the define() needs to be, I believe, directly before the define() line, not before the if() block.

Ah [finally looked at the code, not just the patch].

I guess both of them should have docblocks, within the if/else respectively. I guess we should also still see if the API module will recognize them inside or outside the ifs.

Yes, you're right. The API module allows for multiple "objects" (functions, defines, classes, etc.) to have the same name within the same project, but only one per file (the URL is composed of project / file / object name).

So let's just document it on one (inside the if() I think?), but document what it does for Windows and non-Windows in the same docblock.

That looks quite reasonable to me, thanks!

Looks good to me, committed/pushed to 8.x. I think we could backport this.

D7 patch.

Same patch; definitely needs port. Thanks!

YIKES. The commit to 8.x accidentally included the patch from #375397: Make Node module optional but we're handling the rollback in that issue.

This will need to be reapplied for D8 (see #375397-125: Make Node module optional). ;)

Commit of #11 had to be reverted:
http://drupalcode.org/project/drupal.git/commit/256a10dd192cab26c5c919b4...

One more time, please! :)

Was reverted in 8.x. Needs to have ONLY the patch in #11 committed to 8.x.

Let's try that again. Committed/pushed to 8.x.

Committed and pushed to 7.x. Thanks!

Yay, no sneak API changes this time! ;)

Reopening for some minor followup--syslog_facility_list() has a wrong verb tense, as does SyslogTestCase::testSettings(), and SyslogTestCase is missing a docblock entirely.

I don't think agentrickard has worked on this for a while, so unassigning. Also tagging.

Addresses #22. Changed verb tense and added a docblock to SyslogTestCase

Thanks @kid_icarus! That looks good. One minor thing:

+++ b/core/modules/syslog/syslog.testundefined
@@ -5,6 +5,9 @@
+ * Functional tests for syslog.

This summary should start with a verb as well.

The summary now starts with a verb :) I chose "Represents", as I think that's an accurate verb for a class description.

+++ b/core/modules/syslog/syslog.test
@@ -6,7 +6,7 @@
- * Functional tests for syslog.
+ * Represents functional tests for syslog.

Hm. I would agree with you in Java, but I'm not sure this class really "represents" anything -- it's just a collection of test functions.

Perhaps following the model on other test classes in Drupal Core would be more sensible? Such as:

/**
 * Tests the Color module functionality.
 */

And otherwise, this patch looks fine! I checked and nothing else but these few changes needs to have docs fixes. Of course, the module only has about 25 functions all together. :)

Yeah, you're right on the class not representing anything :) I was influenced by http://drupal.org/node/1354#classes in that decision.

+++ b/core/modules/syslog/syslog.test
@@ -6,7 +6,7 @@
 /**
- * Represents functional tests for syslog.
+ * Tests the Syslog module functionalality.
  */

D'oh, I misspelled functionality as "functionalality".

+++ b/core/modules/syslog/syslog.test
@@ -6,7 +6,7 @@
 /**
- * Tests the Syslog module functionalality.
+ * Tests the Syslog module functionality.
  */

Thanks! Patch in #32 committed to 8.x and 7.x. I think we are officially done cleaning up syslog. :)