UC Clear Roles

Please provide a link to your sandbox project.

The only thing in the repository is the .info file, which has only the name of your project. You need to push your code to the repository. Also it looks like you need to identify yourself to git. The author of your initial commit is unknown

Hi vensires,
You should run your module through Coder with the minor setting selected. This will help you fix some problems that I noticed. After doing that, set this back to Needs Review, and I'll go through and do a more thorough review.

Hi,

this seems great, is it still in development?

Hope you're well,

Evelyn

Vensires,

thanks for getting back to me. I'll check it out right away. Thanks for your hard work!

Evelyn

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Review of the master branch:

• README.txt is missing, see the guidelines for in-project documentation.
• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Go and review some other project applications, so we can get back to yours sooner.

manual review:

• uc_clear_roles.info: Remove "; $".
• "return array(uc_clear_roles_ADMIN);": permissions string should be enclosed in quotes ''. Same in hook_menu().
• "'#options' => user_roles()," you need to sanitize the role names before outputting them to avoid XSS attacks.
• uc_clear_roles_admin_form(): you should use system_settings_form().
• uc_clear_roles_action_order_delete(): for building the query you should use db_placeholders(), see also http://drupal.org/writing-secure-code . And do not use sprintf(), the DB API will handle the replacements for you.

@vensires has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

You need to set the status to "needs review" if you want to get a review.

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 :-)

Please document the requirements and dependencies on your project page. I had to read the .info file to see what it needs to work.
t() shouldn't be used in the hook_menu titles.
Also found a few coding standard issues. Please use the following tool to validate your project: http://ventral.org/pareview

Please don't remove the security tag, we keep that for statistical purposes and to show examples of security problems.

Thanks for your reviews, just make sure that you pick the oldest applications first. 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).

Please add all your reviews to the issue summary as indicated in #1410826: [META] Review bonus.

manual review:

1. "variable_get('uc_clear_roles_roles'...": if you define your own variables you should remove them on module uninstallation (hook_uninstall()).
2. why did you remove comments in the code? Please add them back. The coding standards only say that inline comments should be on there own line and that longer function descriptions should have their own paragraph in the doc block. Removing comments really hurts my feelings :-(

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

.install

7 /**
   8  * Implements hook_install().
   9  */
  10 function uc_clear_roles_install() {
  11 }

Don't implement hooks that do nothing.

Thanks for your contribution!

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.