TinyPass

Welcome,

please get a review bonus first. Then try to fix issues raised by automated review tools: http://ventral.org/pareview/httpgitdrupalorgsandboxtinypass1537700git

Apart from the review: it seems you're using a non-individual account.
All user accounts are for individuals. Accounts created for more than one user or those using anonymous mail services will be blocked when discovered. See Get a Drupal.org account. Please change your account information and enter your realname.

There is an error on page "TinyPass configuration for tags/terms":

JQuery_ui_add() undefined

I ran PAReview: As expected based on earlier comments, the online PAReview came back with no errors.

Good job responding to a user question within the module issue queue. Likely when the module is live, you’ll be checking the issue queue more often though.

You may want to put the tpl.php files within a “theme” folder in order to organize the files more.

My observations that you will want to follow up on:
• Good commenting where you comment but I would appreciate more comments since it’s hard for a developer to understand flow and purpose of each piece, especially when there’s, for example, more than 100 consecutive lines of code without comments.
• In tinypass_tag_list.tpl.php you have logic that could be cleaned up somewhat. Instead of using endforeach, you should use curley brackets around what’s within the loop. If you could pull out more of the logic into the module file, into a preprocess function that would be better, although the logic here isn’t much.
• In tinypass_node_options.tpl.php, you define the $st variable. You should be doing those kinds of simple definitions from within a preprocess function.

Overall, I think the module is very well written. It's just that the separation between business logic and theming is something that needs a little work.

In addition to toniomans hints please consider reworking your template files.
Think of the poor theme who doesn't know PHP and simply tries to alter the markup.

Example (tinypass_tag_list.tpl.php:

• Create the link in a preprocess function (see Setting up variables for use in a template). Btw.: is any user allowed to access the path? Maybe its better to not print the link if the acting user is not allowed to view its resulting page.
• The whole table can be printed using theme_table. Create the variable (e.g. "$tag_list") in the preprocess function mentioned above and simply print this variable in this table. This would be much easier to understand for a person who does not know php. Maybe a comment "@see theme_table" would be great then ...

Is there a small tutorial/handbook about how to test the module? I guess TinyPass offers something like development sandboxes so a tester do not need to buy content for real?

Hello,

1- Your glt clone link in project issues must be -

git clone --recursive --branch 6.x-1.x http://git.drupal.org/sandbox/tinypass/1537700.git tinypass

The one which you have provided asks for a password and it may annoy the reviewers and may stop expert busy reviewers reviewing your project.

2- Add http://git.drupal.org/sandbox/tinypass/1537700.git this git link in the review summery as well.

3- I think '#description' => t('') is not necessary, as you dont want to provide any description. Line #485 in tinypass.module

4- Line #501, @$form['#node']->tinypass->node_meta['meta_id'] doesn't seems to be a proper way of doing it, however I think we can wait for experts review on this part, if this is allowed.

same with - @$po_data = $node->tinypass->node_meta['data']['pos'];

5- You may use check_plain instead of htmlspecialchars(stripslashes($caption)), check_plain in drupal filters the content for safe output.

Thanks

3rd review

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

fixed git URL