Bits on the Run

welcome,

I see you're already working on your coding style issues detected by autmated tools, great! :)

You're readme already looks pretty good but your project page could be better.

Maybe you should put all assets (images, css, js) into a misc directory (just to have a clean module root).

You can also get a review bonus and we will come back to your application sooner.

regards

Hello

I would recommend the following changes :

1) botr.install

I will no put html tags in translatable content.

'description' => $t("The Bits on the Run plugin works better when the <a href='@curl_url'>PHP cURL library</a> is installed.",
31 array('@curl_url' => 'http://php.net/manual/en/book.curl.php')),

could become :

$message = $t("The Bits on the Run plugin works better when the !url is installed.", array('!url' => l($t('PHP cURL library'), "http://php.net/manual/en/book.curl.php")));

Same issue in botr.module with different description fields.

2) botr.module

Remove empty description

225 '#description' => t(''),

3) botr.module

function botr_uninstall() should be in botr.install. Please move the function.

There are some array aligments that are not by Drupal coding standards:

  $filters['botr'] = array(
    'title'             => t('Bits on the Run quicktag replacement'),
    'description'       => t('Replace video quicktags with javascript embeds.'),
    'process callback'  => 'botr_filter_process',
    'settings callback' => 'botr_filter_settings',
    'default settings'  => array(
      'botr_timeout'      => 0,
      'botr_content_mask' => BOTR_CONTENT_MASK,
      'botr_player'       => BOTR_PLAYER,
    ),
    'tips callback'     => 'botr_filter_tips',
    'cache'             => FALSE,
    'weight'            => BOTR_FILTER_WEIGHT,
  );

It would be better here if you concatenate those strings:

  $path = "players/$video_hash-$player_hash.js";
  $content_mask = $filter->settings['botr_content_mask'];
  $url = "http://$content_mask/$path";

like this:

$path = 'players/' . $video_hash . '-' . $player_hash . '.js';
$content_mask = $filter->settings['botr_content_mask'];
$url = 'http://' . $content_mask . '/' . $path;

I would suggest to keep the name "Bits on the Run" instead of "Bits on the Run Module For Drupal".

Otherwise the code looks good.

Hello ,

Manual Review :
api.php:
1. If possible, use your module name as prefix to file name.ex:botr_api.php
2. Please add module/file name as prefix to the functions to avoid name clashes.ex:api_call.
3. Please try to avoid "private" and use "protected" instead.
4. Also variable prefixed with underscore _ is discouraged. so try to avoid this.ex:$_url.

botr.module :
1.you are using l() to create link markup, its pretty good.
2.you can append variables with the string, $url .= "?exp=$expires&sig=$signature" can be $url .= "?exp=" . $expires . "&sig=" . $signature;
3.If possible, don't put any HTML into t() functions.
4.It seems, there are lot of errors in PARREVIEW.Please check it.

Thanks

botr.install:

30 'description' => $t("The Bits on the Run plugin works better when the !curl_link<a href='@curl_url'></a> is installed.",
31 array(
32 '!curl_url' => l($t('PHP cURL library'),
33 'http://php.net/manual/en/book.curl.php'),
34 )),

The <a> tag here looks a little redundant.

49 $result = db_query('SELECT name FROM {variable} WHERE name LIKE "botr|_%" ESCAPE "|"');
50
51 while ($row = db_fetch_object($result)) {
52 variable_del($row->name);
53 }

Drupal best practice is to specify the exact variable names, rather than using 'LIKE', as you can not guarantee that another module isn't using a similarly named variable. This scenario has become more common with the number of modules in contrib which extend other modules. While the likelyhood of a conflict with your particular case may be low, we do encourage using a consistent best practice across all modules.

Otherwise, looks good!

Thanks for your contribution, botr! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

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.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.