Media Mover SNS

Code review result

FILE: /var/www/drupal-7-pareview/pareview_temp/mm_sns.module
--------------------------------------------------------------------------------
FOUND 9 ERROR(S) AFFECTING 9 LINE(S)
--------------------------------------------------------------------------------
110 | ERROR | Concatenating translatable strings is not allowed, use
| | placeholders instead and only one string literal
265 | ERROR | Case breaking statements must be followed by a single blank line
269 | ERROR | Case breaking statements must be followed by a single blank line
285 | ERROR | Array indentation error, expected 4 spaces but found 6
297 | ERROR | Array indentation error, expected 6 spaces but found 8
308 | ERROR | Case breaking statements must be followed by a single blank line
312 | ERROR | Case breaking statements must be followed by a single blank line
364 | ERROR | Function comment short description must be on a single line
398 | ERROR | Function comment short description must be on a single line
--------------------------------------------------------------------------------

My comments

110 | ERROR | Concatenating translatable strings is not allowed, use
| | placeholders instead and only one string literal

The problem is in the '#description' value.
I had to break the line to meet the 80 chars limit.
I don't see a better way to do that without causing other errors.

function mm_sns_admin_form(&$form_state = array()) {
  // ...
  // some code here
  // ...

  $form['mm_sns']['mm_sns_aws_sdk_dir'] = array(
    '#type' => 'textfield',
    '#default_value' => variable_get('mm_sns_aws_sdk_dir', ''),
    '#title' => t('AWS PHP SDK directory'),
    '#description' => t('Enter the AWS PHP SDK directory name within the'
      . ' default library directory. eg. aws_php_sdk_1'),
  );

265 | ERROR | Case breaking statements must be followed by a single blank line

case 'storage':
      return mm_sns_media_mover_storage($op, $action_id, $configuration,
        $file);

I had to break the line to meet the 80 chars limit. That's why the line right after the case-break statement (ie. "return") isn't blank.

It also applies to similar messages.

285 | ERROR | Array indentation error, expected 4 spaces but found 6

From Drupal Coding Standards (http://drupal.org/coding-standards#array):

Note that if the line declaring an array spans longer than 80 characters (often the case with form and menu declarations), each element should be broken into its own line, and indented one level:

364 | ERROR | Function comment short description must be on a single line

/**
 * Configuration form when using the action that just publishes a message on
 * Amazon SNS during the upload "storage" or "complete" step.
 */

Is it a short description? What's a short description definition?

Good point, thanks!

I'm going to implement the hook_uninstall()