TheInspector [theinspector]

Thanks for the interest in contributing to the community!

In order to proceed reviewing this application, please add the module as attachment so we can review it:

You must submit a finished, working module or theme for review along with your application. Upload an archive containing the code to review in the issue automatically created for you by the application. Our review will check for adherence to security best practices, usage of Drupal APIs, and coding standards compliance.

Make sure to have read and understood these two pages:
http://drupal.org/node/59
http://drupal.org/node/539608

Hi,

In a module like this, where there are plenty of modules that do something similar, I believe you should provide a more explicit definition of what the module does and how it does it. Also, a demo site would be great to have.

Here are some things I've found reviewing the code, please revise http://drupal.org/node/539608 to see which one of them could be blocking.

• Please, revise your info file, the header should be simply ;$Id$ and it shouldn't include so much information, check this for the info guideline: http://drupal.org/node/231036
• Same header information goes for the module file, check this http://drupal.org/node/546 for how to implement the Id headers for each file, js files and css files should be corrected as well
• There are some coding standard issues with the code, please use coder module to correct most of them: http://drupal.org/project/coder
• I think you should secure simplechat_post_message function as it gets information directly from $_POST
• In the hook_nodeapi call, you are only switching by $op but not for node type, this means that that code is executed in the view & load op of every node of every content type, this could be a performance issue.
• There is some commented code that could drive to confusion if someone looks at the files.
• README.txt file is required with information about what the module does and how to install and configure it to get it working
• What are you using comment-wrapper.tpl.php for? Please bear in mind that a template with the same name exists in drupal core http://api.drupal.org/api/drupal/modules--comment--comment-wrapper.tpl.p...

Thanks!

Hello, and thank you for applying for a CVS account. I am adding the review tags, and some volunteers will review the code, pointing out what it needs to be changed.

Hi,

I'm not an expert, so treat the following comments carefully!

1) I see you use "variable_set()". I think this means you also need to do a clean-up by creating a simplechat.install file implementing hook_uninstall(), http://api.drupal.org/api/drupal/developer--hooks--install.php/function/...

2) I don't think you need $id$ in the README.txt

3) I think you need to add the ability to make your module themable. This is to HTML what t() is to translators. Lines like the following I guess cannot easily be themed:

$output .= "<p>" . t('No messages unpublished.') . "</p>";

Check out:
http://drupal.org/node/306
http://drupal.org/node/165706

Hi. Please read all the following and the links provided as this is very important information about your CVS Application:

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:
Migrating from CVS Applications to (Git) Full Project Applications

• The status of this application will be put to "postponed" and by following the instructions in the above link, you will be able to reopen it.
• Or if your application has been "needs work" for more than 5 weeks, your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.

As per previous comment, I am setting this issue as Won't fix.
Since new users can now create full projects, applications have a different purpose and they are handled on a different issue queue. See Apply for permission to opt into security advisory coverage for more information.