Hello!

Comment Tree is the name of my module.

I saw the Presentation on Models for hierarchical data and I thought it would be really beneficial to have a module that would store the comment tree in a closure table. This opens up the possibility of other modules to implement sorts on comment threads that are different than the default sort, but in a way that does not require multiple queries to be executed. This module should increase the performance for sites using alternate comment sorting.

The Module works by using hook_schema to define the comment_tree schema. Then it uses hook_comment_insert, hook_comment_update, and hook_comment_delete to maintain the tree. I also created a Batch processes (using the Batch API) to generate the comment tree if a site already has comments. When you enable the module, hook_enable will check to see if you have comments already, if you do, it will give you an error message that you need to generate the comment tree.

Here is a link to my project: https://drupal.org/sandbox/davidwbarratt/1850024

This module is intended only for use in Drupal 7.x (for right now).

Please let me know if you have any problems, I'd be happy to fix them.

thanks!
david barratt

Comments

nesta_’s picture

nesta_ commented
Status: Needs review » Needs work

Remove "version" from the info file, it will be added by drupal.org packaging automatically.

http://ventral.org/pareview/httpgitdrupalorgsandboxdavidwbarratt1850024git

davidwbarratt’s picture

davidwbarratt commented
Status: Needs work » Needs review

Removed!

anything else?

thanks!
david

gazoakley’s picture

gazoakley commented

Hi davidwbarratt,

I ran your project through the automated project review tool here:

http://ventral.org/pareview/httpgitdrupalorgsandboxdavidwbarratt1850024git

There's quite a few Drupal Sniffer errors resulting from things like whitespace at end of line and other minor things. Just for tidyness sake you might want to go through and fix them - they're easy fixes!

gazoakley’s picture

gazoakley commented
Status: Needs work » Needs review

Manual review:

Your module has a configuration page but no configuration link. Drupal 7+ allows modules to specify links for their configuration and doing so is recommended for usability: http://www.drupalcoder.com/blog/configure-links-on-modules-overview-page...

There's a warning for the use of drupal_set_message in comment_tree.install - whilst I can't see any way of XSS there because of the way l() and t() are used you might want to wrap the call in filter_xss() just to get rid of the warning :-)

Can you please check/fix the above issues? Once done change the status to "needs review"

gazoakley’s picture

gazoakley commented
Status: Needs review » Needs work
davidwbarratt’s picture

davidwbarratt commented

okie dokie!

I fixed the drupal_set_message error and I also fixed all the coding standard errors. It appears that there's a clean bill of health now! :)

http://ventral.org/pareview/httpgitdrupalorgsandboxdavidwbarratt1850024git

Anything else I can fix?

thanks!
david

WZavala’s picture

WZavala commented
Status: Needs review » Reviewed & tested by the community

It looks great and works even better!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

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

jthorson’s picture

jthorson commented
Status: Reviewed & tested by the community » Fixed

A few nit-picky comments:

i) I suspect you mean 'descendant' instead of 'decedent' in your module.
ii) Inconsistent capitaliztion in the drupal_set_message() within function comment_tree_enable() (file comment_tree.install), as well as in all docblocks within comment_tree.module. The docblocks may be used to generate pages on sites such as api.drupalcontrib.org, so we recommend regular sentence-case capitalization and punctuation. Also, comment lines can go up to 80 characters before wrapping ... your lines wrap around 40 characters, which could result in display funkyness on api sites.
iii) I believe your batch operation comment could be run through t().

No showstoppers, so ...

Thanks for your contribution, davidwbarratt!

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:

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.

davidwbarratt’s picture

davidwbarratt commented

Thank you so much!

I followed your recommendations and committed the changes.

I've made the project a full project and setup a development and full release.

thanks again!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.