Who Sends API Integration

Automated review

Several errors, see http://ventral.org/pareview/httpgitdrupalorgsandboxdarrenwh1894978git-7x-1x

Manual review

1. Module configuration page comes up with errors, configuration is broken:
   

   Notice: Undefined index: whosends_form in drupal_retrieve_form() (line 763 of /Users/ain/NetBeansProjects/Drupal7/includes/form.inc).
   Warning: call_user_func_array() expects parameter 1 to be a valid callback, function 'whosends_form' not found or invalid function name in drupal_retrieve_form() (line 798 of /Users/ain/NetBeansProjects/Drupal7/includes/form.inc).

2. mail() documentation ln 27-29 in newmailer.class.inc is not standard.

Please fix the issues so the module can be thoroughly tested.

Make sure to change the project status to needs review when you think it's ready to be reviewed again.

Don't forget to add the "PAReview: review bonus" tag as indicated in #1410826: [META] Review bonus, otherwise you won't show up on my high priority list.

Manual review:

Really no problems found. Clean, well documented and, above all, elegant. I am curious, though, why you add an asterisk at the end of your @file blocks. I'm surprised the automated review doesn't choke on this especially since there's a space after each of the asterisks.

Leaving status as is.

Actually, looks like I should change the status to rtbc since I found no issues.

Thank you for your reviews. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no major flaws).

manual review:

1. project page is too short, see http://drupal.org/node/997024
2. who_sends_api_integration_help(): this is a hook implementation and should be documented as such, see http://drupal.org/node/1354#hookimpl . Same for who_sends_api_integration_mail_alter(). Do not copy the hook documentation!
3. "'access arguments' => array('access administration pages'),": The administration menu callback should probably use "administer site configuration" - which implies the user can change something - rather than "access administration pages" which is about viewing but not changing configurations.
4. "variable_get('whosends_api_url', 'https://www.whosends.com/api/getmsgaccess'),": all variables provided/used by your module need to be removed in hook_uninstall().

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

manual review:

1. "variable_get('whosends_api_url', 'https://www.whosends.com/api/getmsgaccess'),": all variables defined by your module need to prefixed with your module's name to avoid name collisions with others. Also elsewhere.
2. "file_get_contents($url, ...": That will not work on many systems, because getting remote files is probably forbidden. Use drupal_http_request() instead

.

But otherwise looks RTBC now. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Thanks for your contribution, darrenwh!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

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.

Thanks to the dedicated reviewer(s) as well.