[D7] NoSpam

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxAmazingDreams2089117git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

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

Hi AmazingDreams,

I checked your module code and ran the pareview.sh on your module. As you have mentioned there are a number of issues reported. I would like to suggest that you should probably fix these as much as possible.

I also did a manual review of your module and would like to suggest the following:

On file 'nosmap.module' for 'nospam_form_admin', as I pick from your code the form is meant for module configuration, so would be better if you use return system_settings_form($form)instead of just retrurn $form, this would save you from implementing the corresponding form submit hook, which drupal already provides for such forms.
Please have a look at the following URL:
https://drupal.org/node/206761

Also as per the convention use variable names in lower case. An implementation example would be as below:

$form['enqueue_chance'] = array(
  '#type'          => 'textfield',
  '#title'         => t('Enqueue chance (%)'),
  '#description'   => t('The chance a users IP has to be enqueued, use this to limit the amount of IP addresses enqueued'),
  '#default_value' => variable_get('enqueue_chance', 10),
);

You can add a default value if you like, the would guide the user trying to do the settings.

On file 'nosmap.module' typo on line 208.

On file 'nospam.module' line no. 274, the API URL is hard-coded, what if it changes?, please consider putting it to module configuration settings, and use variable for it.

On file 'nosmap.module' function 'nospam_set_ip_verified' line no. 344 there is no issue with the 'switch-case' used but for a boolen condition it is better to use 'if'.

On file 'nosmap.module' for function 'nospam_build_query_string' please consider using a combination of array_map and explode, the for loop can be avoided.

Hope this helps.

With the above comments setting it to needs work.

No you do not need to delete the doc block you can just use 'Implements '. Please go through the following link.
https://drupal.org/node/1354

A quick question. How does this module differ from https://drupal.org/project/spambot that also implements the same web service you have used for this module?

The spambot module (I am actually using) provides a lot of the same functions of frequency checks, cron job with configuration of the number of people to check per cron hit, etc.. If you haven't duplicated work it might be best to try and contribute to the spambot module so there is only one module for stopforumspam.com's service.

Hi

You possibly do not need to use variable_set in your implementation of hook_install. You are already calling system_settings_form($form) on line 88 of the .module file. This will create all the system variables for you and you can set the defaults within the the form.

Also you will need to implement a hook_unistall() in your .install file to remove these variables. you can call variable_del to get this done https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/vari....

Thanks
T

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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