[D7] Chatwee

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

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.

1 .Please complete all errors stated in here: http://pareview.sh/pareview/httpgitdrupalorgsandboxpaulqbera2189799git
- You may use https://www.drupal.org/project/coder and drush dcs your.module so you can pin-down most of errors before "Repeat Review" on PAReview.sh

2. Your project page don't have "Similar projects and how they are different" section list which can help other trying to decide for your module.
Example:

• http://drupal.org/project/drupalchat
• http://drupal.org/project/chatroll
• http://drupal.org/project/chatroom
• More from here -> https://www.drupal.org/search/site/chat

Please take some time to make your project page follow tips for a great project page.

-Albert

Master Branch

Installation is easy, works fine. Code seems to be well-formatted, however you should provide all php files with licensing header. All drupal modules should run under GPL license.
Everything else is quite fine and I think this module should be approved to the full project after adding license and headers.

rayzzz.com, please see here: https://www.drupal.org/node/2237645#comment-9005395

Moving this back to "Needs review."

Hi @paul_q

For this review I will follow the format here.

REVIEW for Chatwee

1.2 Ensure your project is not a duplication.
YES

1.3 Ensure you don't have multiple applications.
YES

2. Basic repository checks

2.1 Ensure the repository actually contains code.
YES

2.2 Ensure you are working in a version specific branch.
YES

3. Security Review

3.1 Ensure the project does not contain any security issues.
YES ref: Drupal Security Checks (Checks for security issues. This is a very basic security check. It errs on the side of caution so may give false positives.)

4. Licensing checks

4.1 Ensure the repository does not contain a ‘LICENSE.txt’ file.
YES

4.2 Ensure the repository does not contain any 3rd party (non-GPL) code.
YES

5. Documentation checks

5.1 Ensure the project page contains detailed information.
YES

5.2 Ensure the repository contains a detailed README.txt.
YES

5.3 Ensure the code contains a well-balanced amount of inline-comments.
YES but there is not much code there

6. Coding standards and style

6.1 Run an automated review and ensure there are no major issues.
YES

7. API and best practices Review

7.1 Ensure you are using Drupals API correctly.

(*)NO not enough code to ...
ref: How much code do we need to approve a user?

I propose two simple rules that can be used to deny the "create full projects" permission:
* The project has less than five function or class method definitions
* The PHP/Javscript source code is less than 120 lines long (including comments and whitespace).

Suggestions: Maybe embellish admin/config/services/chatwee a bit?

The starred items (*) are fairly big issues and warrant going back to Needs Work.

@dbt102: small projects are perfectly fine, we just can't give the git vetted user role away based on them. However, we can promote this single project manually to a full project for the applicant.

Looks like you did not read through the source code of the project, you did not list any blocking issues?

@paul_q, thankyou for your contribution.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. None.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

No: Follows the guidelines for project length and complexity. Will proceed with single project promotion.

Secure code

Yes. If "no", list security issues identified.

Coding style & Drupal API usage

1. chatwee.install: Why you require following code, please add your comment on this.
   

   function chatwee_install() {
     user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array('using chatwee'));
   
     user_role_grant_permissions(DRUPAL_AUTHENTICATED_RID, array('using chatwee'));
   }
   
   

2. chatwee_permission(): Did not find, where you have used following permission in your module.
   

   'administer chatwee' => array(
         'title' => t('Administer Chatwee'),
         'description' => t('Permission to change Chatwee settings.'),
       ),
   

   From description it indicates you want to use this permission for admin/config/services/chatwee that need to be fixed.

3. Instead of using error_log("Invalid Chatwee script");, better to log error using Database logging that comes with drupal core.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

Not seeing anything blocker from my end, switching to RTBC.

As I am not a git administrator, so I would recommend you, please help to review other project applications to get a review bonus. This will put you on the high priority list, then git administrators will take a look at your project right away :-)

Thanks Again!

Non-blocking issues:

• Typo in the README, "yoru" should be "your"
• It's pretty unusual to assign permissions to roles in chatwee_install(), generally you just provide the permission in hook_permissions, and let admins configure roles how they like. Consider at least providing a drupal_set_message() that lets the admin know what just changed.
• In chatwee_preprocess_page() do you really want to inject your chat script on all drupal pages, even admin pages? it would also be good to make sure the user has permission to 'use chatwee' on this site.

Checked for licensing, duplication, individual account, Drupal API and security, no major issues found. As noted above, this module is too short to grant "git vetted user" status, so just promoting this 1 project to https://www.drupal.org/project/chatwee.

Thanks for your contribution, paul_q!

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 stay 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.