[D7] AddThis Smart Layers

I guess this needs review? See the project applications workflow.

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

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 welly,

Here are some improvements, few of them are critical as per what I think.
Improvements in .module file

1. Each function should have proper comments stating what that function does.
2. The return statement should be before closing brackets in hook_block_view, so you don't need to repeat that line for each block. See code sample below
3. prefer using drupal_add_js for adding internal/external JavaScript
4. Remove commented code from .module file.

Improvements in .admin.inc file

1. Use meaning full names for variables, $k, $v should not be used.
2. Always use t() with placeholders, to set '#title'
3. I think, the $services array defined in addthis_smart_layers.admin.inc file is wrong, the key should be used as value and vice versa

<?php
function MODULE_NAME_block_view($delta = '') {
  switch($delta) {
    case '$DELTA_CASE1':
      $block['subject'] = t('Block1 Title');
      $block['content'] = custom_block_contents();
      break;
    case '$DELTA_CASE2':
      $block['subject'] = t('Block2 Title');
      $block['content'] = custom_block_contents();
      break;
  }

  return $block;
}
?>

Yogesh

You should update the sandbox project URL and clone url properly.

The URL to clone the project is
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/welly/2145355.git addthis_smart_layers

and the sandbox project URL is
https://drupal.org/sandbox/welly/2145355

Manual Review:

addthis_smart_layers.info

• You should include core and description values.
• It should be ideal to specify the configuration link as well. Refer https://drupal.org/node/1111212.

addthis_smart_layers.module

• Include menu description for settings link, it will list in configuration main page.
• Remove the unwanted t() and place comments on meu hook. Use coder module to easily identify these type issues.

addthis_smart_layers.install

• Create addthis_smart_layers.install file and specify script to remove the created variables while configure this module.

Thank you for your reviews. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no major flaws).

The Git commits are not connected to your user account. You need to specify an email address. See http://drupal.org/node/1022156 and http://drupal.org/node/1051722

Review of the 7.x-1.x branch:

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: /home/klausi/pareview_temp/addthis_smart_layers.module
  --------------------------------------------------------------------------------
  FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   16 | WARNING | A comma should follow the last multiline array item. Found: )
  --------------------------------------------------------------------------------
  

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.

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

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.

Duplication is not longer an application blocker. Changing back to Needs Review.

Indeed, although we still recommend collaboration over competition. @welly: please reopen this if you want to continue with this application.

A am a co-maintainer of AddThis.

If Willy wants to go ahead with this application, I'll be happy to have it as a sub-module of, or integrated with, AddThis. I assume that completing the project so that it is part of AddThis will "count" as a valid application just as much as completing it as a stand-alone module.

However, it needs work. According to this issue: #2599464: Addthis Module and Smart layers conflict, Addthis Smart Layers currently conflicts with AddThis. If Willy continues his application, I'll work with him to resolve this conflict.

If the applicant wants to work with me on making this part of AddThis, please follow-up here.

Dupe (some hickup during submitting).