[D7] Helponclick Live Chat

welcome

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:

• Remove "version" from the ./helponclick_live_chat.info file, it will be added by drupal.org packaging automatically.
• Remove "project" from the ./helponclick_live_chat.info file, it will be added by drupal.org packaging automatically.
• Remove "datestamp" from the ./helponclick_live_chat.info file, it will be added by drupal.org packaging automatically.

while waiting for an in-depht review of your module you can start out fixing some coding style issues detected by automated tools:http://ventral.org/pareview/httpgitdrupalorgsandboxbnorbi1993672git

We do really need more hands in the application queue and highly recommend to get a review bonus so we will(/can) come back to your application sooner.

Updating status

Hi, bnorbi

The README.txt doesn't contain a basic overview of what your module does and how someone may use it.
The contents of the file may be a repeat of the synopsis on your project page.

Hi there!

Automatic review: The code is clean.
Manual review:

• Maybe you forgot to push your changes of the README.txt. The content is still only a placeholder text with no information about your actual project at all.
• .info: You should add an information that the 3rd party provider is actually helponclick.com
• .install: Why do you implement hook_requirements() and hook_schema() if you don't do anything in these functions?
• .module: doc block for helponclick_live_chat_settings_form() is wrong. See: http://drupal.org/node/1354#forms

Actually I don't really get what your module does, which you couldn't achieve (even better) with a couple of clicks with the native block system? Please explain what extra functionality you get by using your module instead of the block system.

I still don't see how your module integrates this special service. You can basically copy / paste any embed code into your embed text field. As long as it contains the string "helponclick" somewhere in the markup it will be accepted as a valid embed code and will be displayed on the website.

I could see some kind of integration if you would provide a textfield where the user enters a code provided by the "Helponclick Live Chat" website and your module would generate the appropriate embed code. At the moment your module doesn't seem to do anything useful, which couldn't be done better by using a standard block (apart from checking that the string "helponclick" is part of the saved content). Sorry I don't want to be rude, but that's just my opinion.

I agree with Stefan, it would be better to use a plain block. Blocks already have a method for setting visibility on various regions/pages/urls/users.

In fact, that's what the docs on http://www.helponclick.com/live-chat-software/integration/drupal.html describe.

If you feel strongly that this module would make it easier to use your service, perhaps it could define a default block and use the same embed code? That would integrate a little better with Drupal.

Code too short

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.

----
Top Shelf Modules - Enterprise modules from the community for the community.