Tag Sphere

You have more to work on Coding standards. Please run your code through Coder.

I have attached the list of task you need to improve in the coding. Please work on it more.

Module duplication is a massive problem on Drupal.org and for many reviewers the first item on a review is whether this functionality currently exists within Drupal.org.

It may be worth writing within your project summary in what way you feel that this module differs from http://drupal.org/project/cumulus or you may get tagged as creating a duplicate module.

Hello guys,

I guess this module differs from the one raynimmo suggested (culumus) in two ways :
#1. Cumulus depends on Tagadelic module.
#2. Cumulus module requires a JS script called SWFObject

And I tried the module by veeraprasad and it is dependent only on jquery plugin. Looks promising.

Manual Review

Repository Branches

Your repo is a bit messed up, everything seems to reside within the 'master' branch with two tags created for 6.x-1.0 and 7.x-1.0. Also, when I cloned your module onto my dev machine it contains a folder labeled tag_sphere and within that are your module files, but you also have a folder within that labelled as 7.x-1.x which contains copies of the same files again!

You should really have separate branches for the 6 & 7 versions, labelled as 6.x-1.x and 7.x-1.x and your master branch should be empty apart from a README.txt file explaining what the module is and where to get it from.

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

README.txt

You actually dont have a README.txt anywhere in either branch. Please take a moment to make your README.txt follow the guidelines for in-project documentation.

tag_sphere.info

The 'version' tag is not required.

tag_sphere.js

You are currently using $(document).ready to fire off your javascript, you should maybe have a look into implementing Drupal.behaviors

Automated Review

The automated review tool at is highlighting the same issues i have mentioned above but it is also complaining about whitespace at the end of a number of lines, have a look at the review page at http://ventral.org/pareview/httpgitdrupalorgsandboxveeraprasad1344492git for further information about which lines need the whitespace removed.

Make sure to set this back to needs review once you have addressed the issues raised.

It would also be worthwhile to run your code through the scanner at http://ventral.org/pareview before you next set it for review and then when it is clear you can post the results to the thread to make reviewers aware that it passes those tests.

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

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

Manual review:

You need delete the array() param on L19 in tag_sphere.install.
Please check the API for variable_del.

Hi veeraprasadd, You should also change the tag to "PAReview: review bonus" as it is written here: http://drupal.org/node/1410826

Now add an issue tag to your application: "PAReview: review bonus" (this can only be done via a comment).

manual review:

1. tag_sphere_init(): why do you need hook_init()? You should only add your Javscript if you actually display a tag sphere on that page? hook_init() is bad for back end performance as it is invoked on every single page request and this is bad for front end performance as you are adding Javascript that might not even be needed.
2. tag_sphere.info: why is there no dependency to the taxonomy module?
3. "'info' => t('!vocab_name', array('!vocab_name' => 'Tag Sphere -' . $val['name'])),": the static string "Tag Sphere -" should not be concatenated with the dynamic variable, it should be directly in the translatable string. Please read the documentation of t() again.
4. "module_load_include('inc', 'tag_sphere', 'tag_sphere.admin');": no need to use module_load_include as you are including files from your own module which you already know where they are. Use something like require_once 'mymodule.inc';
5. tag_sphere_block_view(): why do you need the inline javascript here? That should all be moved to a dedicated JS file and the settings should be passed down from PHP to JS with Drupal.settings. See http://drupal.org/node/756722
6. README.txt: links to download the jquery plugin are confusing: the first did not even load, the second showed no available downloads. I had to manually download the raw JS files from the bitbucket repository.
7. tag_sphere_block_view(): this is vulnerable to XSS exploits. If I have a vocabulary with the title <script>alert('tags Vocab');</script> I get a nasty javascript popup as soon as the block is displayed. You need to sanitize user provided input before printing, please read http://drupal.org/node/28984 again.
8. tag_sphere_block_save(): why can't you simply use "$tag_sphere_settings[$delta] = $edit" instead of all those duplicating lines?
9. tag_sphere.admin.inc: why is this file called *.admin.inc? It does not contain administrative functionality? Better call it tag_sphere.pages.inc?
10. tag_sphere_form_alter(): doc block is wrong, this is not hook_form_FORM_ID_alter() which you should be really using. Then you don't need the form ID check in the function body.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

@klausi: Reviewed other projects and updated summary in first post.

please don't remove the security tag, we keep that for statistics and to show examples of security problems.

manual review:

1. tag_sphere_install(): No need to set variables upon installation as you can make use of default values with variable_get() anyway.
2. tag_sphere_taxonomy_vocabulary_insert(): %name is not a variable, so the text should be driectly included in t().

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.

Assigning to jthorson as he might have time to take a final look at this.

Cool, back to RTBC.

no objections for more than a week, so ...

Thanks for your contribution, veeraprasadd!

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.