This project creates a new Testimonial node in Drupal.
The site admin can add Testimonial for the sites or product in this type.
Once added they can seen as a block which can be put on the site as required

https://drupal.org/sandbox/abbass786/2086245

Comments

PA robot’s picture

Status:Needs review» Needs work

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.

abbass786’s picture

Status:Needs work» Needs review

I cleaned up the code according to the errors shown.

testimonial_form is a hook for type testimonial.
Hence cannot prepend it with the module name.

Please review my Module and let me know your views on it.

Regards,
Abbas.

topsitemakers’s picture

Status:Needs work» Needs review

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.

topsitemakers’s picture

Status:Needs review» Needs work

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

abbass786’s picture

Hi,

Thanks for the deailed review.I have made changes based on the Review comments.
I will also take care about the commit style guidelines given for all commits from now on.

I have created a separtate branch as suggested called as 7.x-1.x and done all my push there.

Following are list of changes based on the review comment
1.)Added Install and congfiguration details in Readme.txt
2.)Removed all commented lines from .info file
3.)Separated ending curly brace of each function from next comment with a new line
4.)Added more text in the hook_help() implementation
5.)Removed all commented code
6.)Added a link to the testimonial node when displayed in blocks.
7.)Some other clean up based on Review comments.
8.)Not deleting the content at the uninstall hook.

Even though the functionality can be replicated by view this module will be useful as with absolute no knowledge or configuration required the testimonials can directly displayed by the user on his Drupal site.
This module makes displaying Testimonials on the site very easy.

Please let me know your views on this.

Regards,
Abbas.

abbass786’s picture

Hi,

Please let me know your views on the above.

Regards,
Abbas.

Enxebre’s picture

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!

Enxebre’s picture

Status:Needs review» Needs work
PA robot’s picture

Issue summary:View changes
Status:Needs work» Closed (won't fix)

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.