Background Audio

welcome,

you got some coding style issues (See automated report at http://ventral.org/pareview/httpgitdrupalorgsandboxonjtharpe1371850git, you can use this site to re-test your self).
If you got any questions on that, please ask!

Before switching back to needs review you should at least move to a version specific branch, create a README.txt and remove wrong file[] lines from your .info (See link above).

Unfortunately I don't think that your module is large enough to review your skills seriously, it would be great if you'd add some more functionality. Also there are quite few comments, think about commenting your code more detailed so we can follow you thoughts.

You say it is a drupal 7 module but your using hook_install/uninstall like in drupal 6. Please read about the use of hook_schema in the documentation for d7.

/**
* Implements hook_schema
**/

should be

/**
 * Implements hook_schema().
 */

  variable_set('Path', $form_state['values']['Path']);
  
  db_update('background_audio')
      ->fields(array(
          'path' => variable_get('Path'),
  ))
  ->execute();
  $var = variable_get('Path');
  variable_set('drupvar', $var);

You have to prefix variables with your module name, also I don't understand the sense of setting and getting variables that often. Can you explain what you want to do here?

regards

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

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

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. Go and review some other project applications, so we can get back to yours sooner.

FILE: ...sites/all/modules/pareview_temp/test_candidate/background_audio.install
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 8 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

Source: http://ventral.org/pareview - PAReview.sh online service

@klausi sry i didn't know, was my bad.

@Ciraxis: please do not set the issue to "needs work" if there are only that view code style errors and you do not give a manual review.

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

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

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. Go and review some other project applications, so we can get back to yours sooner.

You can use http://ventral.org/pareview/httpgitdrupalorgsandboxonjtharpe1371850git to repeat the review.

Manual review of 7.x-1.x branch:

background_audio.info

• Provide a config path in the .info file, such as configure = admin/config/system/background_audio

background_audio.install

• The .install file is empty, you should or remove it or provide some functions, such as hook_enable()

background_audio.module

• 4: Provide a better description in the @file
• 14: In the background_audio_help() check the double spaces in the help text
• 37: Use lowercase keys for $blocks
• 53: I think the height and width should be an setting, instead of hardcoded
• 65: Your $delta is recent, but it should check your block delta, in this case 'Background_Audio' and the variable_set() is not right either.
• 76: Why are you using a textfield for a file field, I think you should use the http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.... and give the users an upload field.
• 100: You can directly return drupal_get_form() instead of using it through the variable $form

README.txt

• Provide a config path in the README.txt
• Provide a useful README.txt, see http://drupal.org/node/161085#readme and http://drupal.org/node/447604 - Style section for some great examples

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

Changed branch.