MailControl

Hi,

• Remove "version" from the info file, it will be added by drupal.org packaging automatically.
• See http://ventral.org/pareview/httpgitdrupalorgsandboxreszli1846498git for an automatic review of the code of your module.

I'm going to do a manual review of your module soon.

Looks good to me, and works.

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Sorry, but this module is not really working.
After enabling it the email subjects and bodies are not saved anymore. This is because you move the fields into fieldsets and do not set the variable name within the new structure. Drupal translates the structure then to variables named "0" and "1" and saves the email texts into this new variables.

Example:

<?php
$form['email_admin_created']['settings'][] = $form['email_admin_created']['user_mail_register_admin_created_subject'];
?>

On submit Drupal (especially system_settings_form_submit extracts a variable called "0" (because you are adding $form['email_admin_created']['user_mail_register_admin_created_subject'] to an array without specifying a key).

Instead doing the above try this:

<?php
$form['email_admin_created']['settings']['user_mail_register_admin_created_subject'] = $form['email_admin_created']['user_mail_register_admin_created_subject'];
?>

This will save the email subjects and body values in the correct variables.

Hi,

mailcontrol.info
remove line 6-7 if there is no dependencies[].

Modifying the form structure of a core form is not a good idea in general.
You are moving the subject and body fields into a container.
Other modules may want to alter the same form, and will not find the elements in their desired position.
This could cause a conflict between your and some other modules.

I think you should apply the #states separately on the fields, without moving them.

Hi reszli,

A had made the code review and all looks ok for me now.Good luck with your module.

fixed git repo link

manual reivew:

• mailcontrol_uninstall(): not sure you should delete those variables since they are not strictly provided by your module.
• This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.

Thanks for your contribution, reszli!

I promoted your sandbox to a full project: http://drupal.org/project/mailcontrol

Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.

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.