[D7] e-Commerce Mailcheck

Welcome,

Try to get review bonus and get your code reviewed faster :)

First you should check and clear your code. More information here:
http://ventral.org/pareview/httpgitdrupalorgsandboxleewillis771609046git

I'll try to look into code soon. Or maybe someone else will do it sooner :)

Regards

I see no commerce module dependency here. If this module works with drupal commerce module, this dependency has to be added into .info file.

don't forget to switch back to needs review if your ready

Thanks for your contribution. I have done a onceover of your code, and I have a few notes:

e_commerce_mailcheck.module, Line 21:

// Drupal Commerce support.
  if (defined('COMMERCE_ROUND_NONE')) {
    drupal_add_js(drupal_get_path('module', 'e_commerce_mailcheck') . '/js/commerce.js');
  }

The if statement to check for Drupal Commerce is unnecessary. Your code implements hook_commerce_checkout_router() which is only called by the Drupal Commerce - Checkout module.


/js/commerce.js
Your JS here does not conform to Drupal standards for JavaScript.
Instead of Document.ready, Drupal namespaces jQuery and uses Drupal behaviors. This ensures that your JS is run on page load, and attaches to anything added to the page via AJAX. Please see http://drupal.org/node/756722

Lastly, this is a great JavaScript library and I'd love to see it in not only commerce, but the user login, registration, and profile forms. Thanks again!

Drupal.behaviours also provides a means to detach behaviours, not just attach

http://drupal.org/node/224333#drupal_behaviors

Another point about Drupal.behaviours is that it's the official API for this. Other module developers expect to be able to call Drupal.attachBehaviours() and Drupal.detatchBehaviours(). If your behaviours aren't properly namespaced, there's a possibility that they won't be handled correctly.

Trying to think of an example which would illustrate this, but see
#350035: Implement Drupal.detachBehaviors()
#862522: Resizable textarea behavior does not detach

Manatwo makes a good point about MailCheck being useful elsewhere. It could be applied to any email field, but I suppose public-facing forms are the main use case.

Would you consider making this more generic, rather than specifically a commerce add-on? I'll do some coding to help if you like.

I'm thinking a mailcheck.module could handle the library integration, and a drush command to fetch it fom github. Also messages for post-install and the status page. (commerce_mailcheck.module would have this as a dependency.)

Other developers could add support for other use cases as stand-alone projects. Example: webform_mailcheck.module would glue this onto webform email fields.

The MIT license for mailcheck.js may make it suitable candidate for the Drupal.org Library Packaging Whitelist, although the status of the so-called MIT license is a bit vague.

I would say this should go into #1535548: [D7] Mailcheck. Please collaborate with that maintainer to add that as a feature there. Feel free to reopen if this was a mistake.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2288335

Project 2: https://www.drupal.org/node/1609068

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.