Part of meta-issue #1310084: [meta] API documentation cleanup sprint

This is for the system module, subdirectory tests, files and sub-directories in there starting with J through Z, including the themes and upgrade subdirectories.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

Title: Clean up API docs for simpletest/tests, J-Z » Clean up API docs for simpletest/tests, J-Z, including subdirectories
xjm’s picture

Title: Clean up API docs for simpletest/tests, J-Z, including subdirectories » Clean up API docs for system/tests, J-Z, including subdirectories
xjm’s picture

Issue summary: View changes

Added explicit mention of subdirs

mmartinov’s picture

Assigned: Unassigned » mmartinov

I'm taking this, I should be able to post a patch soon.

Albert Volkman’s picture

Status: Active » Needs review
FileSize
17.37 KB

Here's a first rough patch.

jhodgdon’s picture

Status: Needs review » Needs work

Thanks!

The standard for menu callbacks is:
http://drupal.org/node/1354#menu-callback
- You need : instead of ;
- The next thing should be a verb like "Records the active trail for 403/404 pages" rather than "Used for ..."

So, most of this patch needs an update.

Albert Volkman’s picture

This patch resolves the semicolon versus colon mixup. Next up is fixing the verbiage.

Albert Volkman’s picture

Re-roll of #6.

Albert Volkman’s picture

Status: Needs work » Needs review
FileSize
643 bytes
11.4 KB

Fixed incorrect docblock for system_test_basic_auth_page().

jhodgdon’s picture

Status: Needs review » Needs work

Thanks, looking better!

+++ b/core/modules/system/tests/modules/module_test/module_test.module
@@ -93,7 +93,7 @@ function module_test_menu() {
 }
 
 /**
- * Page callback for 'hook dynamic loading' test.
+ * Page callback: For 'hook dynamic loading' test.

These do not follow our standards for hook_menu callbacks:
https://drupal.org/node/1354#menu-callback
They need a verb. There are 4 in this file that need updating, one in session_test.module, and a couple in theme_test.module.

Other than that, the patch looks pretty good... Oh, here's another problem, in session_test.module:

+ * Page callback: Attempts to save a value with after disabling session.
  */
 function _session_test_no_set($value) {

"with after"? This doesn't quite make sense. :)

And can we fix the typo here:

+++ b/core/modules/system/tests/modules/url_alter_test/url_alter_test.install
...
+/**
  * Impelement hook_install().
Albert Volkman’s picture

Give this a try.

Albert Volkman’s picture

Status: Needs work » Needs review
Albert Volkman’s picture

Issue summary: View changes

.

jhodgdon’s picture

Issue summary: View changes
Status: Needs review » Closed (won't fix)

These issues are a lot of work with very little tangible payoff, so I'm closing the rest of them as "won't fix". Your efforts on working on this issue were appreciated... it was just my fault for starting a task that was very difficult to get right.

Let's instead put our effort into fixing and reviewing documentation that is really unclear and/or wrong, and I hope that the people who worked on these issues are not afraid to jump into a more reasonable issue!