[D7]TestimonialBlocks

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

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.

General

I am not sure if this module is necessary and useful to the community. It just lists the titles of testimonial nodes and does not even provide a link to the testimonials node/listing page. This functionality could be replicated using Views quite easily.

However, more people will need to comment on this here so I will proceed with the review.

Review

Commit-style

• you should keep more meaningful track of the changes in git, preferably step by step
• Instead of having multiple commits with the same message, you can merge them in one commit so that it makes more sense.
  Otherwise you cannot distinguish what has been done just by looking at the commit message.
  There are two ways of fixing this:
  • enter what exactly has been fixed (e.g. “adding the space after foreach statement and fixing the comment for hook_form() implementation” for this commit)
    OR
  • merge those commits into one; you can do that with the following command while working:
    

    git add .
    git commit --ammend -C HEAD
    

    The last changes will just be added to the last commit and saved together with it.

• You seem to be still working on the “master” branch in git. This should be changed to 7.x-1.x for example. See more information about git branching here: http://drupal.org/node/1127732
• The module should contain some basic information in README. Include installation and usage details there as well as on the project landing page.
  You can check out any other project's description to know what to write about.

Code:

• Remove all uncommented lines from .info file (package, files, dependencies, php) as they are unnecessary
• Separate ending curly brace of each function from next comment with a new line
• Add more text in the hook_help() implementation
• Avoid mixing the single and double quotes. There is a string with double quotes in testimonialblocks.module file on line 38
• Remove all commented code
• Comments should end with a dot

Critical

The module SHOULD NOT delete content type and all nodes in hook_uninstall() explicitly!
Drupal will disable and remove the content type automatically once you uninstall the module because you already declared it in the hook_node_info() implementation.
Deleting the nodes will cause data loss directly - usual practice is to leave them in the database so the site owner can decide what to do with them (either delete manually or convert them to a different content type).

For reference, see .install files of core modules that provide custom content types, e.g. Blog, Forum and Poll.

Let me know if you have any questions regarding the feedback.

Changing the status to "Needs work" based on the feedback above.

Hi abbass786,

There is a call to "print_r" function in .install file.

In .install file, Functions "function _testimonialblocks_installed_fields()" and "function _testimonialblocks_installed_instances()" always return and empty array. I am sorry but I am not able to see its purpose.

In testimonialblocks_block_view it would be a good idea to create the links with "l()" function.

I agree with topsitemakers tha the functionallity of this module may be not enough to create an own module.

Hope this helps.

Regards!

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.