Part of meta-issue #1310084: [meta] API documentation cleanup sprint and continuation of #1327484: Clean up API docs for book.

This issue is focused on further changes to bring Book module closer to D8/D7 documentation standards. This issue, for instance, will ensure that there are no missing @param or @return directives from docblocks and that the various Test files are in accord with http://drupal.org/node/1354. There also appears to be at least one missing @file directive.

Files: 
CommentFileSizeAuthor
#7 book_docs-1807688-5.patch11.29 KBAlbert Volkman
PASSED: [[SimpleTest]]: [MySQL] 39,934 pass(es).
[ View ]
#5 1807688-5-book-docs.patch13.32 KBAlbert Volkman
PASSED: [[SimpleTest]]: [MySQL] 49,304 pass(es).
[ View ]
#5 interdiff.txt4.44 KBAlbert Volkman
#3 1807688-3-book-docs.patch15.45 KBLars Toomre
PASSED: [[SimpleTest]]: [MySQL] 42,825 pass(es).
[ View ]
#1 1807688-1-book-docs.patch922 bytesLars Toomre
PASSED: [[SimpleTest]]: [MySQL] 42,088 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new922 bytes
PASSED: [[SimpleTest]]: [MySQL] 42,088 pass(es).
[ View ]

Attached is a small patch from starting to perform a full review of the Book module for compliance with D8 documentation standards.

If interested, please add to this patch as one does a full review of each of the files in the Book module directory.

Status:Needs review» Active

This patch has no problems, so I committed it to 8.x and the .js file part to 7.x (the css file does not exist there). Thanks! Leaving open for further work if necessary.

Status:Active» Needs review
StatusFileSize
new15.45 KB
PASSED: [[SimpleTest]]: [MySQL] 42,825 pass(es).
[ View ]

Here is a patch that contains changes from doing a full review of the Book module (including its Test class).

Status:Needs review» Needs work

Thanks, most of this looks good! A few updates needed:

a) In book.admin.inc:

- *   The form that is being modified.
+ *   The form that is being modified, passed by reference

Needs . at end of line.

b) in book.install:

/**
- * Move book settings from variables to config.
+ * Move Book settings from variables to config.

Only capitalize Book in "the Book module". I guess here it should say "Move Book module settings...". Also check other capitalization changes, such as in the BookTest.php file.

d) book.module

+ * @return
+ *   A Boolean TRUE if the current user has the requested permission.
+ *
  * @see book_menu()
  */
function _book_outline_access(Node $node) {

By convention we do not include return value docs for hook_menu access callbacks (there are 3 in this patch).

e) book.module

/**
  * Processes variables for book-node-export-html.tpl.php.
  *
- * The $variables array contains the following elements:
- * - node
- * - children
+ * @param array $variables
+ *   An associative array containing the following keys:
+ *   - node

- children got left out - was that intentional?

f) BookTest.php

+   * @param array $nodes
+   *   An array of Nodes to check in outline.
    *
-   * @param array $nodes Nodes to check in outline.
+   * @return string
+   *   A customized regular expression as a string.
    */
   function generateOutlinePattern($nodes) {

Nodes should not be capitalized in the @param description? And the @return docs don't actually tell me what the regexp is for -- and of course it's a string (that is how regexps are in PHP), so that's not useful information.

g)_ same file

+   *   (optional) Parent book reference id. Defaults to NULL.
    */
   function createBookNode($book_nid, $parent = NULL) {

id -> ID

Status:Needs work» Needs review
StatusFileSize
new4.44 KB
new13.32 KB
PASSED: [[SimpleTest]]: [MySQL] 49,304 pass(es).
[ View ]

Addressed issue raised in #4 and found a few others.

Version:8.x-dev» 7.x-dev
Status:Needs review» Patch (to be ported)

Thanks! This one looks good, and I committed it to 8.x. I guess we should port it to 7.x -- careful though, as there are references to the Node class etc. which don't exist in 7.x.

Status:Patch (to be ported)» Needs review
StatusFileSize
new11.29 KB
PASSED: [[SimpleTest]]: [MySQL] 39,934 pass(es).
[ View ]

Assigned:Unassigned» jhodgdon
Status:Needs review» Reviewed & tested by the community

Looks good to me! I'll get it committed soon. (Only one pass on this one -- great!)

Assigned:jhodgdon» Unassigned
Status:Reviewed & tested by the community» Fixed

It's in -- thanks!

Status:Fixed» Closed (fixed)

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