[D7] Better Comments

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.

Automated Review

No. Some major issues reported on PAreview.sh. It seems that default branch is not set so PAreview is not able to review it correctly.

Submitted by Anonymous (not verified) on Mon, 09/22/2014 - 17:59
Git default branch is not set, see the documentation on setting a default branch.
It appears you are working in the "HEAD" branch in git. 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.
Review of the HEAD branch (commit ):

README.txt or README.md is missing, see the guidelines for in-project documentation.
No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.
This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.
Source: http://pareview.sh/ - PAReview.sh online service

Best practice issues identified by pareview.sh / drupalcs / coder. Please don't copy/paste all of the results unless they are short. If there are a lot, then post a link to the automated review and mention that problems should be addressed.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Not sure. Similar module Ajax Comments exists but author already provided explaination about different set of features.: Does not cause module duplication and fragmentation.

Master Branch

Yes/No: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes. But it seems that default branch is not set so PAreview is not able to get README.txt file.: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Need to validate further.. If "no", list security issues identified.

Coding style & Drupal API usage

1. (*) Major finding

   Avoid hook_init. There are multiple efficient ways to include JS / CSS instead of using hook_init. MYTHEME_preprocess_node can be used.

2. Minor finding

   Syntax is incorrect for function hook_form_FORM_ID_alter
   Check correct syntax here https://api.drupal.org/api/drupal/modules%21system%21system.api.php/func...

   Some commented code is there in tpl files.

   hook_help doesnt provide any information about the module.

3. (+) Release blocker
4. (*) Major finding

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

I will play with this module on a Vanilla Drupal installation and let you know in case I get any issues.

Thank you for the contribution Samir!

Automated reviews

PAReview didn't through any errors.

Manual review

1. The variables which are set using the system_settings_form should be deleted in hook_uninstall()in a install file of the module. Naming conventions of the variables follow the standards.
2. You don't need to mention the version in the info file. It gets added automatically by drupal.org packaging script.
3. On the project page and this issue summary (and also in the README file), put a slash "/" (or "or") in between "Disable" and "enable" for clear distinguishing. E.g. "Disable / enable the text filter options below comment box."

I didn't install the module and check the functionality. I'll do it when these issues get resolved. This certainly looks like a useful module.

Git default branch is not set, see the documentation on setting a default branch.

It appears you are working in the "7.x.1.x" branch in git. You should really be working in a version specific branch: 7.x-1.x. 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.

See also: http://pareview.sh/pareview/httpgitdrupalorgsandboxsamirmankar2342293git...

function better_comments_form_comment_form_alter(&$form, &$form_state, &$form_id);

$form_id should be called as pass by value.

Ref: https://api.drupal.org/api/drupal/modules!system!system.api.php/function...

HI,
Thanks for this awesome module.Please have a look on the below mentioned points.
1. i have used coder module for review and got a mini problem in it.(attached in the screen shot)
2. When there is no image assigned then it took extra space in the comment.attached in the screenshot.
3. It will be good if you will give an on/off option to use better option on the form page.

HI,
Thanks for this awesome module.Please have a look on the below mentioned points.
1. i have used coder module for review and got a mini problem in it.(attached in the screen shot)
2. When there is no image assigned then it took extra space in the comment.(attached in the screenshot.)
3. It will be good if you will give an on/off option to use better option on the form page.
4. please review the code on http://pareview.sh/pareview/httpgitdrupalorgsandboxsamirmankar2342293git . and fix all the issues.

Please don't RTBC your own issues, see the workflow: https://www.drupal.org/node/532400.

Please treat these rules with understanding and be patient.

Automated Review

Submitted by Anonymous (not verified) on Mon, 09/22/2014 - 17:59
Git default branch is not set, see the documentation on setting a default branch.
Review of the 7.x-1.x branch (commit be2abf6):

DrupalPractice has found some issues with your code, but could be false positives.
FILE: /var/www/drupal-7-pareview/pareview_temp/better_comments.module
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------
136 | WARNING | Do not call theme functions directly, use theme('image', ...)
| | instead
349 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible
471 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible
--------------------------------------------------------------------------------

Time: 138ms; Memory: 10.5Mb

No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

• Can you not use theme('image', $picture) as it suggests?
• What are you using the blank array $form_state['input'] for?
  Can you not change this to $form_state['values'] = array()?

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Unsure!!!: May cause module duplication and fragmentation.
Perhaps give even further detail of the difference between your module and other comment related contrib modules specifically.
There are many comment modules, what does yours do differently that other modules don't do?
Are none of the features you mention provided by any of the below?

• Comments Plus
• Ideal Comments
• Advanced Comments, etc.

Master Branch

No: You must set a default branch /
follow the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes. If "no", list security issues identified.

Coding style & Drupal API usage

1. (*) Why are some of these 'errors' defined as strings and some as constants. Where are they declared?
   

         function better_comments_preview($form, &$form_state) {
     if (form_get_errors()) {
       $form['#attributes'] = array('class' => array('comment-form', errors));
       $form['#attributes'] = array('id' => array('comment-form', errors));
       $form = drupal_render($form);
       $commands[] = ajax_command_replace('.comment-form', $form);
     }
         

         function better_comments_submit(&$form, &$form_state) {
     if (form_get_errors()) {
       $form['#attributes'] = array('class' => array('comment-form', 'errors'));
         

2. When I add a comment, it shows on the page. When I edit the comment and save it disappears. If I refresh the page, the comment was edited correctly. So, something amiss with the edit display. I used minimal install profile on Garland theme with a simple page content type with comments enabled.
3. If I add a reply comment, when I edit the parent, the parent is updated but the reply disappears. Again, if I refresh the comment shows.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

This review uses the Project Application Review Template.

Hi,

1/ Your callback function better_comments_menu should have parameters $form and $form_state because the function pasted through drupal_get_form.

2/ When I checked all options in the administration form, the render of the comment form went weird (see the file).

3/ One variable_get in administration page isn't named properly.
Line 35 of file better_comments.admin.inc
'#default_value' => variable_get('better_commentss_author', 0),
should be
'#default_value' => variable_get('better_comments_author', 0),

4/ Functions that implements a hook should have in their description Implements hook_NAME
(e.g : better_comments_comment_view())
And you should add parameter details in your custom functions description.

Regards,
Antoine

1. There are too much error PA Review, You need to run your module with Coder module too.
2. Fieldset is empty on admin form.
3. No need of Disable Comment subject textbox setting as there is already with each content type.
4. Show user picture beside comment box options is disabled?
5. No need of Disable text filter format options below comment box. option too. Because we can set the comment body as plain text in Comment Fields Settings.
6. No need of Disable the Preview button setting too as there is already with each content type.
7. I submitted three comments without refreshing the page. Then I refreshed the page. It showing the Comment Posted message thrice. Can you make it happen once?

Better Comments Module Review

Master Branch

Not sure. But it follows the guidline for master branch provided by drupal.org.

3rd party code

It follows the guidelines for 3^rd party code.

README.txt/README.md

Yes. It has the Readme.txt file and following the readme template.

Duplication

There is another same module but that module is providing some different set of features. So it is not violating the duplication terms and conditions.

Individual user account

It provides individual user accounts and is following guidelines provided by drupal.org

Licensing

It follows the licensing requirements.

Secure code

Security is not up to the mark needs some more validation.

Code long/complex enough for review

Module is following the guidelines of code complexity and length.

Help

Hook_help does not provide enough information of the Module.

Code Issues

better_comments.module

• Line 140: String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms [style_string_spacing]

     <div class="comment-user-picture">' .  theme('user_picture', array('account' => $picture)) . '</div>';;

PR review

Here is the PR Review

@samir_mankar

The branch names of your project confuse people. Please, do something with the branch 7.x.1.x:

• rename it to something specific, like new-features
• add it a meaningful suffix, like 7.x-1.x-new-features
• remove it from repository

Assigning to myself for next review.

Automated Review

FILE: /var/www/drupal-7-pareview/pareview_temp/better_comments.module
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------
360 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible
486 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible

Manual Review

Individual user account

[Yes: Follows] the guidelines for individual user accounts.

No duplication

[Yes: Does not cause] module duplication and/or fragmentation.

Master Branch

[Yes: Follows] the guidelines for master branch.

Licensing

[Yes: Follows] the licensing requirements.

3rd party assets/code

[Yes: Follows] the guidelines for 3rd party assets/code.

README.txt/README.md

[Yes: Follows] the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

[Yes: Follows] the guidelines for project length and complexity.

Secure code

[Yes: Meets the security requirements.

This review uses the Project Application Review Template.

*status update*

@jepSter, do you consider the PAReview warnings blocking issues per the comment #40? Did you manually review the usage? PAReview not coming up clean isn't, by itself, a reason to send a project back to Needs Work.

I've took a closer look on your module.

There are dozens of similar ones. A very basic google search: https://www.google.de/webhp?sourceid=chrome-instant&ion=1&espv=2&ie=UTF-...

Your module isn't multi-language ready.

$form_state['values']['comment_body'][LANGUAGE_NONE]['0']['value']

That's bad coding style. You shouldn't work directly with the values and additionally not in that nested level. May you take a look on https://www.drupal.org/node/1797072 and https://www.drupal.org/documentation/entity-metadata-wrappers.

Also your project page is "thin". Please read the following page for an appropriate project page: https://www.drupal.org/node/997024.

Putting your project back to "Needs work".

Reassigning back to myself; I was midway through reviewing this.

(Note it looks like there were updates and reviews since I had assigned this to myself; some of mycomments may be out of date.)

Automated Review

Review of the 7.x-1.x branch (commit 20c2fd3):

• DrupalPractice has found some issues with your code, but could be false positives.
  

  
  FILE: /home/matt/PAR/pareview_temp/better_comments.module
  --------------------------------------------------------------------------------
  FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
  --------------------------------------------------------------------------------
   360 | WARNING | Do not use the raw $form_state['input'], use
       |         | $form_state['values'] instead where possible
   486 | WARNING | Do not use the raw $form_state['input'], use
       |         | $form_state['values'] instead where possible
  --------------------------------------------------------------------------------
  
  Time: 351ms; Memory: 12Mb
  

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

(*) Unsure: Does not cause module duplication and/or fragmentation. Please outline differences between this and other similar projects in the summary and on the project page.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

(*) Looks like $items['better_comments/edit/%/cancel'] should not 'access content' permission, as direct access may lead to access bypass problems.
Maybe 'edit own comments'? It looks like 'access content' is wrong on almost all of those menu items.

Coding style & Drupal API usage

(+) You behavior needs to use the context that was passed to it in the jQuery selectors.

(+) In better_comments_settings(), don't break translated strings across lines. This makes translation difficult. This
is a valid warning to ignore form PAReview.

Rather than including the CSS and JS on every page by specifying it in the .info file, you should use #attached, probably in the comment forms.

The permission on $items['better_comments/%/edit'] looks wrong. You are not defining 'comment_access'

(+) better_comments_form_comment_form_alter implements hook_form_BASE_FORM_ID_alter().

In better_comments_form_comment_form_alter(), I am not sure if LANGUAGE_NONE is appropriate. There should be a proper language set in either $form or $form_state that you can use.

(*) better_comments_reply() shouldn't do the access check. You should define a new access callback for this menu entry, and use that instead.

(+) better_comments_reply() and elsewhere, #markup should avoided; use proper renderable elements.

better_comments_confirm_delete_submit(), the cache_clear_all() is a little extreme. You should just be able to delete the proper entries in {cache_page}

(+) Removed dead code in better_comments_comment_view_alter().

I think the $form_state['input'] warnings can be ignored, as you aren't actually using the unsanitized values anywhere.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

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

This review uses the Project Application Review Template.

@samir_mankar , if/when you think this is ready to be looked at again, set the status to Needs Review.

Assigning to myself for next review, which should be tonight.

Review of the 7.x-1.x branch (commit 5feecc7):

• DrupalPractice has found some issues with your code, but could be false positives.
  

  
  FILE: /home/matt/PAR/pareview_temp/better_comments.module
  --------------------------------------------------------------------------------
  FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
  --------------------------------------------------------------------------------
   356 | WARNING | Do not use the raw $form_state['input'], use
       |         | $form_state['values'] instead where possible
   481 | WARNING | Do not use the raw $form_state['input'], use
       |         | $form_state['values'] instead where possible
  --------------------------------------------------------------------------------
  
  Time: 377ms; Memory: 11.75Mb
  

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

Manual Review

(+) You behavior needs to use the context that was passed to it in the jQuery selectors.

(+) better_comments_form_comment_form_alter implements hook_form_BASE_FORM_ID_alter().

(+) The final if() in better_comments_comment_view_alter() is still dead code. $prefix will never be used; you already used it above;

(*) better_comments_reply() still has access checks and will return menu codes. This should be handled by an access check on the menu item; not in the menu callback itself.

Security issue looks better, but the (*) issue above is a failry big API problem.

And (*) Unsure: Does not cause module duplication and/or fragmentation. Please outline differences between this and other similar projects in the summary and on the project page.

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

We have reviewed (i.e. PR reviews and manual review both) this module earlier, now we have tested again and found following errors:

• better_comments.module
  

  Line 262: missing space after comma [style_comma_spacing]
           'use-ajax', 'button','delete-cancel'),

• better_comments.js

File: @file block missing (Drupal Docs) [comment_docblock_file]

removed automated review.

This sounds like a feature that should live in the existing ajax_comments project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the ajax_comments issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

Differences to the following modules are also missing on the project page:
https://www.drupal.org/project/comment_goodness
https://www.drupal.org/project/comment_plus
https://www.drupal.org/project/ideal_comments
https://www.drupal.org/project/advanced_comment

I'm not sure how yet another comment improving module would help users when there are already so many existing modules out there that can be improved.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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

Review of the 7.x-1.x branch (commit 8147dd0):

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• ESLint has found some issues with your code (please check the JavaScript coding standards). See attachment.
• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. better_comments_submit(): why do you wipe out the form input with "$form_state['input'] = array();"? Please add a comment.
2. better_comments.info: why do you add the comments JS to every single page request? Shouldn't you only add it if actual comment stuff is displayed on the page? Same for the CSS.
3. better_comments.module: you should move out all page callbacks to a dedicated *.pages.inc file so that the module file gets smaller and is easier to maintain.

But that are not critical application blockers, 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 mpdonadio as he might have time to take a final look at this.

Automated Review

Review of the 7.x-1.x branch (commit 8147dd0):

(not reiterating the results from #68)

Manual Review

Agree with comments in #68.

In better-comments.tpl.php, you have a variable_get(). It is best to move that to a preprocess function and pass in a boolean for use in the template.

You have a bunch of variable_get() w/o a default value. It is best to provide one.

Otherwise nothing jumped out at me that would block this application.

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

This review uses the Project Application Review Template.

Thanks for your contribution, samir_mankar!

I updated your account so you can 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 stay 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.