[D7] Comment Smiley

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

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.

Hi Raj,

Manual review:
Project page is too short, see https://drupal.org/node/997024

As you arre getting/setting the module specific variables, these variables should be deleted on the uninstallation of the module.

Can you fix the above issues? I am changing the status to "Need work", you can change it to "Need review" after making the changes.

Thanks

Changing the status as per above comment

Automatic Review

I'll be great if you include a link to pareview:
http://pareview.sh/pareview/httpgitdrupalorgsandboxrajvisu2305701git-7x-1x

It says you must define the default branch. Here's the documentation: https://www.drupal.org/node/1659588

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes I'm not really sure about what exatly this module do, so ... I think it's a new functionallty.

Master Branch

No: It does not have a default branch defined.

Licensing

Yes: It follows the licensing requirements

3rd party code

Yes: I didn't found any 3rd party code so it's ok.

README.txt

No: The readme is very short and not clear. And the project page is not clear too.

Code long/complex enough for review

Yes: a bunch of lines and functions.

Secure code

Yes. : I checked al SQL querys and all of then use de database API. Didn't found any cross site, scripting possibility.

I couldn't do the module works using the README and I'm sure it has much work passing the coding standards.

I forgot the status.

:D

Individual user account
Yes: Follows the guidelines for individual user accounts.

No duplication
Yes.

Master Branch
Yes.

Licensing
Yes.

3rd party code
Yes: I didn't found any 3rd party code.

README.txt
No: The readme doesn't meet the required template at all

Code long/complex enough for review
Yes: quite enough.

Secure code
Yes.

README improvements are surely not an application blocker, any other problems that you found?

Just readme, no other problems found

Module works fine for me.
One issue I found was when we don't enter subject and at the time of display subject get shown from body content which shows text like ":cry::ecry:" in the subject. But this is a small issue and can be taken care later at stable releases.
And one more thing please change name of you image folder from "default" to more appropriate like "icons" or "smiley-img".
Other than this module looks good.

@raj_visu, first of all thanks to you for contributing.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. One minor issue found.

FILE: /var/www/drupal-7-pareview/pareview_temp/comment_smiley.js
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
1 | ERROR | Missing file doc comment

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

(*) Maybe: Does not cause module duplication and fragmentation. Can you follow the steps in the link and confirm that the functionality contained in this module should stay separate from Smiley?.

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 the README Template.

Code long/complex enough for review

Yes: Currently follows the guidelines for project length and complexity. There is a possibility of code reduced once suggested changes will done.

Secure code

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

Coding style & Drupal API usage

1. (+) I tested your module functionality, it worked for me. In case of Filtered HTML, it shows smiley code too. You should mention this thing somewhere in your module either in README.txt file or with description as Text Format should be FULL HTML. See given screenshot for reference.
   
   
2. hook_help is missing in your module.
3. (+) Instead of using jQuery(document).ready, Use Drupal.behaviors.
   

   drupal_add_js('jQuery(document).ready(function () {slogo = ' . drupal_json_encode($arr_img) . '});', array('type' => 'inline', 'scope' => 'footer')
       );
   

4. (*) In following code, you rendered image using separate template file (comment-smiley-logo.tpl.php) that is not required.
   

   $output = array(
         'url' => file_create_url($img->uri),
         'alt' => $img->name,
         'class' => '',
       );
       $replace[] = theme('comment_smiley_logo', array('output' => $output));
   

   You can directly use theme_image() function here instead of calling separate theme template as there is only image rendered. It would also help to reduce following code from .module file:

   /**
    * Implements hook_theme().
    */
   function comment_smiley_theme() {
     return array(
       'comment_smiley_logo' => array(
         'template' => 'comment-smiley-logo',
         'variables' => array(
           'output' => NULL,
         ),
       ),
     );
   }
   

5. (+) In comment_smiley.js file, Use Use Drupal.behaviors Instead of using jQuery(document).ready. Also ending semicolon(;) are missing in this js file.

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.

As I am not a git administrator, so I would recommend you, please help to review other project applications to get a review bonus. This will put you on the high priority list, then git administrators will take a look at your project right away :-)

Thanks Again!

Why is this module comment-specific? Shouldn't we use a module that can apply smileys as text filter to any text format?

https://www.drupal.org/project/smiley
https://www.drupal.org/project/smileys

This sounds like a feature that should live in the existing smiley project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the smiley 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".

https://www.drupal.org/project/smiley supports smiles in comments w/o the WYSIWYG. You just need to assign the filter to a text format, and then allow that text format for comments. I tested this.