https://drupal.org/sandbox/stephmcglathery/2106681

I needed for the user to have the ability to enter a custom email-address in the UI for receiving user registration notifications. So, I created this module.

You can modify the email address here: /admin/config/system/site-information

Git: https://github.com/stephmcglathery/returnPathD7

CommentFileSizeAuthor
#6 pareview_result.txt2.68 KBAjitS
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tr33m4n’s picture

Status: Needs review » Needs work

Hello there,

Please fix the problems found through the automated review process http://pareview.sh/pareview/httpgitdrupalorgsandboxstephmcglathery210668... , most of which are Drupal coding standard formatting issues

You also need to use a version specific branch rather than master as described here https://drupal.org/node/1127732

Could you also update your git url to the one provided by Drupal for ease of review? eg http://git.drupal.org/sandbox/stephmcglathery/2106681.git

Thanks

ChristianAdamski’s picture

Hey,

I'm still waiting for my application as well so I read through the guidelines. They state, that this issue queue is intended to "prove" you are able to write good code. Which kinda requires a certain amount of code. This modules has barely 20 lines of actual code.

And the code itself, again with all due respect, while sure fine for your specific usecase, seems unfinished for widespread, universal usage.

stephmcglathery’s picture

Status: Needs work » Needs review

Fixed the spaces. Committed and pushed. I also created and switched to branch: 7.x-1.x

PA robot’s picture

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.

AjitS’s picture

Status: Needs review » Needs work

Automated review:

There are still issues reported by pareview which needs to be addressed. http://pareview.sh/pareview/httpgitdrupalorgsandboxstephmcglathery210668.... Please check for the attachment.

Manual review:

  1. As mentioned by @ChristianAdamski, and according to the Project Application Checklist:
    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, that means we can't make sure you're able to write secure code and use Drupal's APIs correctly.
    If this is the case, we can't approve you as git vetted user (giving you the permission to create full projects). Either we can see that you have the needed knowledge by other contributions within the drupal community or we can promote it to a full project manually.
  2. Your form is adding a field to the system_site_information_settings form, which sets a variable pending_account_notifications_email. You should have an pending_account_notifications.install file and implement hook_uninstall in it and delete this variable using variable_del(). Don't expect your users to delete the variables (as you've instructed in README.txt) on uninstalling when the API can do that.

General comment

I see less LOC as a major concern in getting you to the "vetted" user access. However, I see this as an useful functionality. I would suggest you to add some more features to the project, some sort of configurations could help. Or look into ways in which this functionality could be added to any module out there.

Best,
Ajit

AjitS’s picture

FileSize
2.68 KB

Missed to attach the pareview results.

PA robot’s picture

Issue summary: View changes
Status: Needs work » Closed (won't fix)

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.