ActiveHelper LiveHelp

Hi Peter,

There is an online tool which automatically performs a review of your code against the Drupal coding guidelines. When reviewing your code with this tool, it generates a large amount of issues.

You can find the tool at http://ventral.org/pareview/
The results of the automatic review of your code can be found at http://ventral.org/pareview/httpgitdrupalorgsandboxactivehelper1691848git

Can you please fix these issues first?

Hi Peter,

You say that you fixed the issues reported by pareview, but when I run it through the application again I get a massive list of remarks. I agree that many of these are related to wrong indentation or too many whitespaces, but these things are important in the Drupal community. If everybody uses the same coding standards, it is a lot easier to review code.
So please have a look at the coding standards at http://drupal.org/coding-standards and have a look at the comments made by pareview at http://ventral.org/pareview/httpgitdrupalorgsandboxactivehelper1691848git and try to fix them.

I have also had a look at your code, and we'll need some time with that. You have a lot of code (over 7000 lines)... Anyway, something that I would already like to mention is that you sometimes use "LIMIT" in SQL queries. This is actually a statement which is not standard SQL syntax and not every database supports it. Drupal does support it through its database abstraction layer, have a look at db_query_range().

Also, you don't have to add a license.txt file. The Drupal packager automatically adds it for you when you'll be creating releases later.

You can also get a review bonus and we will come back to your application sooner: http://drupal.org/node/1410826

Maybe something wrong with git updates, maybe you push to another git repo. The last commit was on 2012-07-31 01:03:35. Try to:
git clone your drupal repo
git clone --recursive --branch 7.x-1.0 activehelper@git.drupal.org:sandbox/activehelper/1691848.git activehelper_livehelp
make code changes and then git commit, git push.

Creating global variables looks like not a Drupal way.
Pleas, check this, maybe it will help you to improve your module.

$render = '
		<div class="livehelp-navigation">
			<a href="' . url( 'admin/activehelper-livehelp/domains' ) . '">
				<img src="' . $activehelper_livehelp['images_url'] . '/world-32.png" />
				<span>' . t( 'Domains' ) . '</span>
			</a>
		</div>
		<div class="livehelp-navigation">
			<a href="' . url( 'admin/activehelper-livehelp/agents' ) . '">
				<img src="' . $activehelper_livehelp['images_url'] . '/user-chat-32.png" />
				<span>' . t( 'Agents' ) . '</span>
			</a>
		</div>
		<div class="livehelp-navigation">
			<a href="' . url( 'admin/activehelper-livehelp/chats' ) . '">
				<img src="' . $activehelper_livehelp['images_url'] . '/up-two-bars-32.png" />
				<span>' . t( 'Monthly chats' ) . '</span>
			</a>
		</div>
....

Please, pay attention on theme() function and change you module accordingly. Maybe it will be even better to use template for admin/activehelper-livehelp page.

$block[ 'content' ] = '
			<script type="text/javascript" src="' . $activehelper_livehelp[ 'import_url' ] . '/javascript.php"></script>

Including js files like this is not a good practice check Managing JavaScript in Drupal 7 for more info.

What is this archive file "activehelper_livehelp_external_original.zip" for?

Closing due to lack of activity. Feel free to reopen if you are still working on this application.