Learning Registry

If this is for review then you should change the status to 'needs review' and not 'active'

I have only had a quick scan over your repository and code and noticed a few points.

README.txt

You have a README.md ? shouldnt that be a README.txt?

Overuse of t()

• Do not use t() in hook_schema(), this will only generate overhead for translators.
• Do not use t() in your #title and #description fields in your learning_registry_menu(), as it is not required as translation of menu items is deferred until runtime.

Automated Review

• README.txt is missing, see the guidelines for in-project documentation.
• Comments: there should be a space after "//", see http://drupal.org/node/1354#inline
  

  learning_registry.module:180:  //watchdog('learning_registry', json_encode($empty_submission));
  learning_registry.module:184:  //watchdog('learning_registry', $resource_post_payload, array(), WATCHDOG_DEBUG);
  learning_registry.module:387:  //TODO: extract these foreach calls into their own function
  learning_registry.module:482:  //TODO:  Refactor candidate.  This method has two responsiblities:  a.) to determine if the node was published to
  

• ./learning_registry.content.inc: comment lines should break at 80 characters, see http://drupal.org/node/1354#general
  

  // http://www.lullabot.com/articles/drupal-5-making-forms-that-display-their-own-results
  

• ./learning_registry.install: comment lines should break at 80 characters, see http://drupal.org/node/1354#general
  

    // http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/variable_del/6#comment-10934
  

• ./learning_registry.module: comment lines should break at 80 characters, see http://drupal.org/node/1354#general
  

    //watchdog('learning_registry', $resource_post_payload, array(), WATCHDOG_DEBUG);
    //TODO:  Refactor candidate.  This method has two responsiblities:  a.) to determine if the node was published to
    // the Learning Registry, b.) then to update that status to the learning_registry table.  The first step should stand on its own.
        // Check if this has been published before.  If no, set the published timestamp.  Else,
            // Current impl of LR spits two title tags, so we're grabbing the last one
            // as it displays better error message.  If not two present, we'd just grab
    // Pretty format the XML before sending it to the LR.  Makes it easier to read and debug.
  

• Comments should be on a separate line before the code line, see http://drupal.org/node/1354#inline
  

  ./learning_registry.module:33:} // function learning_registry_help
  ./learning_registry.module:44:} // function learning_registry_perm
  ./learning_registry.module:380:    'tos_submission_attribution', // need to figure out more about this value.
  

• ./learning_registry.content.inc: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
  

  
  function _learning_registry_content_form_values($form_values = array()) {
  

• ./learning_registry.install: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
  

  
  function learning_registry_enable() {
  

• ./learning_registry.module: The description on the line after the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
  

  342-
  344- */
  

• All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
  

  ./learning_registry.admin.inc ./learning_registry.content.inc ./learning_registry.info ./learning_registry.install ./learning_registry.module
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

When you are running Coder make sure that it is set on 'minor' to catch some of these, alternatively download Klausi's script, drop it into the root of your site and run it by entering paraview.sh sites/default/modules/learning_registry > learn_reg_test.txt and then check the text file it creates for errors.

Good luck with the rest of your review.

The automated review script is still finding a few errors within your module.

It is also not reading your README.txt file as it does not have a file type attached to it. After checking your repository at http://drupalcode.org/sandbox/jasonh/1342822.git/tree/refs/heads/6.x-1.x I noticed it is simply called README and you have omitted the .txt.

Now that you have created your 6.x-1.x branch you should empty out your master branch.

Automated Code Review

• README.txt is missing, see the guidelines for in-project documentation.
• ./learning_registry.module: comment lines should break at 80 characters, see http://drupal.org/node/1354#general
  

    // watchdog('learning_registry', $resource_post_payload, array(), WATCHDOG_DEBUG);
        // Check if this has been published before.  If no, set the published timestamp.  Else,
            // Current impl of LR spits two title tags, so we're grabbing the last one
            // as it displays better error message.  If not two present, we'd just grab
    // Pretty format the XML before sending it to the LR.  Makes it easier to read and debug.
  

• ./learning_registry.module: The description on the line after the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
  

  343- */
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

There may still be some areas unchecked as a manual review will be required once you can satisfy the automated code reviews.

RE: Automated Code Reviews
Before commiting you should run your code through the Coder module with the settings at 'minor'. There are also a host of optional add ons that run with Coder and help to spot errors in your code such as Coder Tough Love, the Translation Template Extractor and Text Review to name some of the more popular ones. There is also a really good script called PAReview.sh that does a great job of spotting errors, its still a sandbox project though so is liable to changes.

Hi, some deeper check with drupalcs enabled:

Automated review (Please keep in mind that this is primarily a high level check that does not replace but, after all, eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.)

Review of the 6.x-1.x branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards):
  

  Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
  
  sites/all/modules/pareview_temp/test_candidate/learning_registry.module:
   +48: [minor] Comment should be read "Implements hook_foo()."
   +52: [normal] Menu item titles and descriptions should NOT be enclosed within t().
   +53: [normal] Menu item titles and descriptions should NOT be enclosed within t().
   +62: [normal] Menu item titles and descriptions should NOT be enclosed within t().
   +63: [normal] Menu item titles and descriptions should NOT be enclosed within t().
   +76: [minor] Comment should be read "Implements hook_foo()."
   +110: [minor] Comment should be read "Implements hook_foo()."
   +126: [minor] Comment should be read "Implements hook_foo()."
   +331: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
  
  Status Messages:
   Coder found 1 projects, 1 files, 5 normal warnings, 4 minor warnings, 0 warnings were flagged to be ignored
  

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
  

  
  FILE: ...es/all/modules/pareview_temp/test_candidate/learning_registry.admin.inc
  --------------------------------------------------------------------------------
  FOUND 5 ERROR(S) AFFECTING 5 LINE(S)
  --------------------------------------------------------------------------------
    31 | ERROR | Closing brace indented incorrectly; expected 2 spaces, found 4
    93 | ERROR | Line indented incorrectly; expected 4 spaces, found 6
    97 | ERROR | Line indented incorrectly; expected 6 spaces, found 8
   100 | ERROR | Closing brace indented incorrectly; expected 8 spaces, found 10
   119 | ERROR | Closing brace indented incorrectly; expected 2 spaces, found 4
  --------------------------------------------------------------------------------
  
  
  FILE: .../all/modules/pareview_temp/test_candidate/learning_registry.content.inc
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   11 | ERROR | Missing space after the // 
  --------------------------------------------------------------------------------
  
  
  FILE: ...ites/all/modules/pareview_temp/test_candidate/learning_registry.install
  --------------------------------------------------------------------------------
  FOUND 10 ERROR(S) AND 4 WARNING(S) AFFECTING 8 LINE(S)
  --------------------------------------------------------------------------------
   10 | WARNING | Format should be * Implements hook_foo().
   18 | WARNING | Format should be * Implements hook_foo().
   23 | ERROR   | Multi-line assignments must have the equal sign on the second
      |         | line
   23 | ERROR   | An operator statement must be followed by a single space
   25 | ERROR   | Multi-line assignments must have the equal sign on the second
      |         | line
   25 | ERROR   | An operator statement must be followed by a single space
   28 | ERROR   | An operator statement must be followed by a single space
   28 | ERROR   | There must be a single space before an operator statement
   28 | ERROR   | Closing brace must be on a line by itself
   30 | ERROR   | An operator statement must be followed by a single space
   30 | ERROR   | There must be a single space before an operator statement
   30 | ERROR   | Closing brace must be on a line by itself
   36 | WARNING | Format should be * Implements hook_foo().
   47 | WARNING | Format should be * Implements hook_foo().
  --------------------------------------------------------------------------------
  
  
  FILE: ...sites/all/modules/pareview_temp/test_candidate/learning_registry.module
  --------------------------------------------------------------------------------
  FOUND 57 ERROR(S) AND 11 WARNING(S) AFFECTING 57 LINE(S)
  --------------------------------------------------------------------------------
    48 | WARNING | Format should be * Implements hook_foo().
    76 | WARNING | Format should be * Implements hook_foo().
    80 | ERROR   | Inline control structures are not allowed
   110 | WARNING | Format should be * Implements hook_foo().
   112 | WARNING | Line exceeds 80 characters; contains 84 characters
   126 | WARNING | Format should be * Implements hook_foo().
   136 | WARNING | A comma should follow the last multiline array item. Found: )
   153 | ERROR   | Closing brace must be on a line by itself
   156 | ERROR   | Inline control structures are not allowed
   157 | ERROR   | Space before closing parenthesis of function call prohibited
   158 | ERROR   | Inline control structures are not allowed
   159 | ERROR   | Space before closing parenthesis of function call prohibited
   181 | ERROR   | Multi-line assignments must have the equal sign on the second
       |         | line
   181 | ERROR   | An operator statement must be followed by a single space
   241 | ERROR   | An operator statement must be followed by a single space
   241 | ERROR   | There must be a single space before an operator statement
   250 | WARNING | Line exceeds 80 characters; contains 90 characters
   261 | ERROR   | Inline control structures are not allowed
   263 | ERROR   | Inline control structures are not allowed
   270 | WARNING | Line exceeds 80 characters; contains 92 characters
   275 | WARNING | Line exceeds 80 characters; contains 81 characters
   311 | ERROR   | An operator statement must be followed by a single space
   311 | ERROR   | There must be a single space before an operator statement
   323 | ERROR   | Variable "submissionURL" is camel caps format. do not use
       |         | mixed case (camelCase), use lower case and _
   324 | ERROR   | Variable "submissionURL" is camel caps format. do not use
       |         | mixed case (camelCase), use lower case and _
   409 | ERROR   | Calling class constructors must always include parentheses
   467 | WARNING | Line exceeds 80 characters; contains 81 characters
   495 | ERROR   | Line indented incorrectly; expected at least 6 spaces, found 4
   503 | ERROR   | There must be a single space before an operator statement
   513 | ERROR   | Line indented incorrectly; expected at least 8 spaces, found 6
   515 | ERROR   | Line indented incorrectly; expected at least 8 spaces, found 6
   549 | ERROR   | Line indented incorrectly; expected 2 spaces, found 4
   553 | ERROR   | Line indented incorrectly; expected 4 spaces, found 6
   558 | ERROR   | Line indented incorrectly; expected 4 spaces, found 6
   560 | ERROR   | Inline control structures are not allowed
   562 | ERROR   | Line indented incorrectly; expected 6 spaces, found 8
   568 | ERROR   | Line indented incorrectly; expected 8 spaces, found 10
   571 | ERROR   | An operator statement must be followed by a single space
   571 | ERROR   | There must be a single space before an operator statement
   571 | WARNING | Line exceeds 80 characters; contains 162 characters
   572 | ERROR   | Closing brace indented incorrectly; expected 10 spaces, found
       |         | 8
   573 | ERROR   | Closing brace indented incorrectly; expected 8 spaces, found 6
   574 | ERROR   | Closing brace indented incorrectly; expected 6 spaces, found 4
   575 | ERROR   | Closing brace indented incorrectly; expected 4 spaces, found 2
   629 | ERROR   | Variable "xmlDoc" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   629 | ERROR   | Calling class constructors must always include parentheses
   630 | ERROR   | Variable "xmlDoc" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   633 | ERROR   | Variable "nodeList" is camel caps format. do not use mixed
       |         | case (camelCase), use lower case and _
   633 | ERROR   | Variable "xmlDoc" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   634 | ERROR   | Variable "dcNode" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   634 | ERROR   | Variable "nodeList" is camel caps format. do not use mixed
       |         | case (camelCase), use lower case and _
   637 | ERROR   | Variable "xmlDoc" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   638 | ERROR   | Variable "dcNode" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   641 | ERROR   | Variable "xmlDoc" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   642 | ERROR   | Variable "dcNode" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   645 | ERROR   | Variable "dateCreated" is camel caps format. do not use mixed
       |         | case (camelCase), use lower case and _
   646 | ERROR   | Variable "xmlDoc" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   646 | ERROR   | Variable "dateCreated" is camel caps format. do not use mixed
       |         | case (camelCase), use lower case and _
   647 | ERROR   | Variable "dcNode" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   650 | ERROR   | Variable "xmlDoc" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   651 | ERROR   | Variable "dcNode" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   655 | ERROR   | Variable "returnXML" is camel caps format. do not use mixed
       |         | case (camelCase), use lower case and _
   655 | ERROR   | Calling class constructors must always include parentheses
   656 | ERROR   | Variable "returnXML" is camel caps format. do not use mixed
       |         | case (camelCase), use lower case and _
   657 | ERROR   | Variable "returnXML" is camel caps format. do not use mixed
       |         | case (camelCase), use lower case and _
   658 | ERROR   | Variable "returnXML" is camel caps format. do not use mixed
       |         | case (camelCase), use lower case and _
   658 | ERROR   | Variable "xmlDoc" is camel caps format. do not use mixed case
       |         | (camelCase), use lower case and _
   660 | ERROR   | Variable "returnXML" is camel caps format. do not use mixed
       |         | case (camelCase), use lower case and _
  --------------------------------------------------------------------------------
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Hey Jason! Just did another quick review of the mod...results below. (The Learning Registry looks like a very cool project, btw!)

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
  

  FILE: ...pal/6.x/sites/all/modules/learning_registry/learning_registry.admin.inc
  --------------------------------------------------------------------------------
  FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
  --------------------------------------------------------------------------------
    8 | ERROR | Inline comments must start with a capital letter
   11 | ERROR | There must be no blank line following an inline comment
  --------------------------------------------------------------------------------
  
  
  FILE: ...l/6.x/sites/all/modules/learning_registry/learning_registry.content.inc
  --------------------------------------------------------------------------------
  FOUND 5 ERROR(S) AFFECTING 5 LINE(S)
  --------------------------------------------------------------------------------
     7 | ERROR | Inline comments must start with a capital letter
    12 | ERROR | There must be no blank line following an inline comment
   109 | ERROR | There must be no blank line following an inline comment
   125 | ERROR | There must be no blank line following an inline comment
   135 | ERROR | There must be no blank line following an inline comment
  --------------------------------------------------------------------------------

Manual review:

• Still quite a few array formatting errors; always use 2 spaces per level. See http://drupal.org/node/318#array
• Implement hook_requirements() to check if the PHP cURL and DOM modules are available.
• _learning_registry_content_form_values: change "$link = blah to instead use l()
• learning_registry_install: Use st() instead of t() in hook_install(), hook_uninstall() and hook_update_N()

Looking better...a few more notes/considerations below.

Automated review:

Review of the 6.x-1.x branch:

• ./learning_registry.install: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function curl_requirements($phase) {
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Manual review:

• Per the "curl_requirements" note above -- check out http://api.drupal.org/api/drupal/developer--hooks--install.php/function/... for proper implementation; as it is now, this function won't ever be run.
• Also per the cURL stuff -- it may be worth considering drupal_http_request at some point as well. Discretionary based on performance results, etc. of course.
• learning_registry.module: _learning_registry_set_deleted and _learning_registry_update -- are these empty stubs necessary at this point? I noted the commented calls in learning_registry_nodeapi, but can you explain the future implementation plans for these functions a bit?
• learning_registry_admin_settings -- foreach ($options as $option), indent 2 spaces. This is present in a few other places as well (loops and arrays) ...see http://drupal.org/node/318#indenting.
• _learning_registry_content_form_values -- add check_plain() calls to each of the $form_values['post'][...] vars -- might as well scrub the input before sending it over to the LR.
• Super-picky style note: a bit more consistency with quotes would be good. See http://drupal.org/node/318#quotes

Apologies for the delay on the review...holiday stuffs, you know.

Automated review:

• FILE: ...es/all/modules/pareview_temp/test_candidate/learning_registry.admin.inc
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
  12 | ERROR | No space before comment text; expected "// " but found "//"
  --------------------------------------------------------------------------------
• FILE: .../all/modules/pareview_temp/test_candidate/learning_registry.content.inc
  --------------------------------------------------------------------------------
  FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
  --------------------------------------------------------------------------------
  11 | ERROR | No space before comment text; expected "// " but found "//"
  14 | ERROR | No space before comment text; expected "// " but found "//"
  --------------------------------------------------------------------------------

Just remove the extra comment delimiters or add a space after each flagged instance.

Manual review:

• learning_registry.module, ln 396 -- extra terminator found.

Otherwise, this looks great to me...nice work! Putting the RTBC stamp on it...more experienced reviewers are welcome to jump in, of course.

blockers

// TODO: figure out if we need to validate this information.

I'd go figure that out now. The answer is yes for node_url, and tos urls, which if they are local should be required as node/N format and converted to that format if provided as an alias so that throughout the module you can then always treat them the same way. If they are external, that should also be checked and validated.

Also, the current validation function 'learning_registry_admin_settings_validate' is never called because it is in the admin.inc file, which is not included before Drupal looks for validation functions. The admin.inc and content.inc forms both need working validation.

#option arrays, magic values

This one moved from highly recommended to a blocker when I found that you had magic values all through the code.
In the option arrays in learning_registry_admin_settings, the option arrays should be line wrapped as per the coding standards, and all values should be fixed and specified rather than relying on the magic value of the order they were entered in. eg

    '#options' => array(t('Immediate'), t('Queued')),

becomes

    '#options' => array(
      LEARNING_REGISTRY_QUEUE_IMMEDIATE => t('Immediate'),
      LEARNING_REGISTRY_QUEUE_QUEUED => t('Queued'),
    ),

.. with the appropriate, commented define() calls at the front of the module.

direct access to post variables instead of values

In _learning_registry_content_form_values, there is direct access to the post values instead of using the processed form_state['values'][X] versions.
Also, this function is called even when the values that have been posted have been potentially rejected by the validation function. It will also be called more than once with a non NULL value. And accepts spaces as valid input.

string translations

There are numerous strings that should be translated that are not translated, as well as strings that should parameterised with t() that are not.

commented out debugging code

The module is meant to be release ready - having watching debugging messages commented out in various ways doesn't scream release ready. Here's another good one:

// file_put_contents("/tmp/foo.json", $json);

highly recommended

.gitignore

Remove .gitignore from repository. That is a local settings file for the person using git, not for everyone using the module.

master missing README.txt

Add a README.txt file to the master branch informing the poor sucker who pulled it where they can find the actual code

README.txt

The README in the 6.x-1.x branch needs to be expanded to something similar to the project home page, plus installation instructions if they're not too complex, in which case they would go into INSTALL.txt.

function learning_registry_node_operations

array not indented correctly, includes "disabled" key which doesn't seem to be supported at all. label should be translated.

function _learning_registry_bulk_operations_publish

This function could potentially load every single node in the site and at present, it does not queue the publishing if configured to do so. The potential to run the site out of memory has not been considered - node_load should probably use the static cache reset to ensure that 3 billion nodes aren't loaded, and the entire process should potentially check to see if it should be queuing the processing for later instead of loading each node anyway. messages should be parameterised and translated.

merely suggestions

remove comment from .info file

I don't think I have ever seen a module with a comment in there

finding the maxium of two values

there's a function for that - max(var,var) - learning_registry_enable

learning_registry_content_form - form_values vs form_state

The usual format for this function in Drupal 6 is

function learning_registry_content_form(&$form_state) {

.. and will always be included so will never be set to NULL. In PHP 5.3, the site may get annoyed with the &.

use of curl

The module uses curl - can it use drupal_http_request instead?

limiting cron workload

The publishing of nodes during cron should be limited to a configurable number (batching) so that execution limits are not reached

commented out code

In hook_nodeapi, there's a large chunk of commented out and obviously wrong code. This should be removed or fixed.

_publish, _deleted, _update

All of these functions are passed the fully loaded node, but none of them seem to really need it. Could all of the processing be done with just the nid? Loading a node is not a cheap process and with all the loading than can potentially happen, it might be better to avoid it depending on how often this code gets run.

_learning_registry_queue_node

this function should use drupal_write_record instead of making your own SQL

drupal_http_request is the way that I've always done it. HTML Export uses this method and then ensures that 404/403/500 weren't returned so that's probably a valid way of handling the issue of verification based on potential error code generated.

Don't RTBC your own issues.

You have not listed any reviews of other project applications in your issue summary as strongly recommended here: http://drupal.org/node/1011698

This module has useful, original functionality, and overall the code looks excellent and very well-written! It's been in the review queue for a very long time, and @jasonh has been diligent about responding to feedback above, so I think we should try to move it towards full project status as soon as possible.

I reviewed the latest code in the module and found two issues that might be blockers, because they are potential security issues:

#1517794: learning_registry_content_form_submit() doesn't sanitize data before printing it
#1517796: learning_registry_admin_settings() doesn't sanitize node types before printing them

Once those two are addressed, it's my opinion that this module is ready to be promoted to a full project.

***

While reviewing, I also found a number of smaller remaining issues. These are all worth looking into fixing, but they're basically just garden-variety bugs/tasks so I don't think there's any reason to hold up the project application over these. For completeness, though, here's the list:

#1517732: Code should not call theme_table() directly
#1517740: Code that might execute at runtime should not call st() directly
#1517748: Minor PHPDoc improvements and cleanup
#1517750: Document or remove the node_load() static cache reset in hook_cron()
#1517758: Validation of drupal_http_request() calls isn't fully implemented
#1517762: Consider increasing severity of watchdog() call
#1517766: Stray semicolon
#1517768: PHP notices on the content form
#1517776: drupal_http_request() calls can be made to an invalid URL
#1517780: Both menu items have the same title
#1517784: Form validation function should validate the terms of service URL also
#1517786: Add a short README.txt to the master branch
#1517812: Some drupal_set_message() calls don't use t()

I reviewed the commits and I'm confident the potential security issues raised above are fixed. So I think this is ready to go. Thanks!

You have not listed any reviews of other project applications in your issue summary as strongly recommended here: http://drupal.org/node/1011698

Review of the 6.x-1.x branch:

• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

manual review:
"UPDATE {learning_registry} SET updated=%d": there should be a space before and after the "=". Also elsewhere.

But that are just minor issues, so ...

Thanks for your contribution, jasonh! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.