FB Likebox

you have to post link to your sandbox project:)

I did a quick Coder review to check the Coding Standards, see below.

Also rename the tpl folder to templates and remove the empty fb_likebox.

Severity minor, Drupal Commenting Standards, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards, Internationalization

sites/all/modules/fb_likebox/fb_likebox.module:
+2: [normal] Commits to the Git repository do not require the CVS $Id$ keyword in each file.
+9: [minor] There should be no trailing spaces
+10: [minor] Missing period
+11: [minor] There should be no trailing spaces
+14: [normal] use a space between the closing parenthesis and the open bracket
+22: [normal] use a space between the closing parenthesis and the open bracket
+32: [minor] There should be no trailing spaces
+38: [minor] There should be no trailing spaces
+39: [minor] Missing parenthesis after function name
+40: [minor] There should be no trailing spaces
+41: [minor] There should be no trailing spaces
+42: [minor] There should be no trailing spaces
+45: [normal] use a space between the closing parenthesis and the open bracket
+46: [normal] Control statements should have one space between the control keyword and opening parenthesis
+46: [normal] use a space between the closing parenthesis and the open bracket
+48: [minor] There should be no trailing spaces
+50: [minor] There should be no trailing spaces
+50: [normal] The $string argument to t() should not begin or end with a space.
+53: [minor] There should be no trailing spaces
+55: [minor] There should be no trailing spaces
+58: [normal] curly braces { should end a line, not start one
+70: [minor] There should be no trailing spaces
+73: [critical] Potential problem: hook_block() only accepts filtered text as the block title, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
+73: [normal] The $string argument to t() should not begin or end with a space.
+74: [minor] There should be no trailing spaces
+78: [minor] There should be no trailing spaces
+80: [minor] There should be no trailing spaces
+84: [minor] There should be no trailing spaces
+87: [minor] There should be no trailing spaces
+91: [minor] There should be no trailing spaces
+92: [minor] Missing period
+93: [minor] There should be no trailing spaces
+96: [normal] curly braces { should end a line, not start one
+102: [normal] Arrays should be formatted with a space separating each element and assignment operator
+102: [minor] There should be no trailing spaces
+102: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
+103: [normal] Arrays should be formatted with a space separating each element and assignment operator
+103: [normal] missing space after comma
+103: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
+105: [normal] missing space after comma
+106: [minor] There should be no trailing spaces

sites/all/modules/fb_likebox/tpl/fb_likebox.tpl.php:
+-1: [minor] @file block missing
+1: [minor] There should be no trailing spaces
+3: [minor] There should be no trailing spaces
+4: [minor] There should be no trailing spaces
+5: [minor] There should be no trailing spaces
+6: [minor] There should be no trailing spaces

sites/all/modules/fb_likebox/fb_likebox.admin.inc:
+1: [minor] There should be no trailing spaces
+2: [normal] Commits to the Git repository do not require the CVS $Id$ keyword in each file.
+7: [minor] There should be no trailing spaces
+12: [minor] There should be no trailing spaces
+16: [normal] use a space between the closing parenthesis and the open bracket
+17: [minor] There should be no trailing spaces
+18: [minor] There should be no trailing spaces
+30: [minor] There should be no trailing spaces
+36: [minor] There should be no trailing spaces
+38: [minor] There should be no trailing spaces
+43: [normal] Use an indent of 2 spaces, with no tabs
+44: [normal] Use an indent of 2 spaces, with no tabs
+44: [normal] Arrays should be formatted with a space separating each element and assignment operator
+44: [minor] There should be no trailing spaces
+45: [normal] Use an indent of 2 spaces, with no tabs
+46: [minor] There should be no trailing spaces
+52: [normal] Use an indent of 2 spaces, with no tabs
+53: [normal] Use an indent of 2 spaces, with no tabs
+53: [normal] Arrays should be formatted with a space separating each element and assignment operator
+53: [minor] There should be no trailing spaces
+54: [normal] Use an indent of 2 spaces, with no tabs
+55: [minor] There should be no trailing spaces
+61: [normal] Use an indent of 2 spaces, with no tabs
+62: [normal] Use an indent of 2 spaces, with no tabs
+62: [normal] Arrays should be formatted with a space separating each element and assignment operator
+62: [minor] There should be no trailing spaces
+63: [normal] Use an indent of 2 spaces, with no tabs
+64: [minor] There should be no trailing spaces
+70: [normal] Use an indent of 2 spaces, with no tabs
+71: [normal] Use an indent of 2 spaces, with no tabs
+71: [normal] Arrays should be formatted with a space separating each element and assignment operator
+71: [minor] There should be no trailing spaces
+72: [normal] Use an indent of 2 spaces, with no tabs
+73: [minor] There should be no trailing spaces
+74: [minor] There should be no trailing spaces
+79: [normal] Use an indent of 2 spaces, with no tabs
+80: [normal] Use an indent of 2 spaces, with no tabs
+80: [normal] Arrays should be formatted with a space separating each element and assignment operator
+80: [minor] There should be no trailing spaces
+81: [normal] Use an indent of 2 spaces, with no tabs
+82: [minor] There should be no trailing spaces
+83: [minor] There should be no trailing spaces
+84: [minor] There should be no trailing spaces
+90: [minor] There should be no trailing spaces
+96: [minor] There should be no trailing spaces
+97: [minor] There should be no trailing spaces
+98: [minor] There should be no trailing spaces
+99: [minor] There should be no trailing spaces
+100: [minor] There should be no trailing spaces
+101: [minor] There should be no trailing spaces
+107: [minor] There should be no trailing spaces
+109: [minor] There should be no trailing spaces
+111: [minor] There should be no trailing spaces
+113: [normal] use a space between the closing parenthesis and the open bracket
+117: [normal] use a space between the closing parenthesis and the open bracket
+117: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
+120: [minor] There should be no trailing spaces
+123: [normal] use a space between the closing parenthesis and the open bracket
+126: [minor] There should be no trailing spaces
+128: [normal] use a space between the closing parenthesis and the open bracket

Status Messages:
Coder found 1 projects, 3 files, 1 critical warnings, 43 normal warnings, 63 minor warnings, 0 warnings were flagged to be ignored

double post

Nice work, some minor thing:
In the info file you may remove the version line, it will automatically added by drupal when the download links are generated.

For the rest it looks good to me, I'll ask a second opinion to make sure

nothing you can do, sit back and relax

Seeing code like this in a template should be a red flag to check that those variables are being properly sanitized:

   8 <iframe
   9 src="http://www.facebook.com/plugins/likebox.php?href=<?php echo $fb_url ?>&width=<?php echo $fb_width ?>&colorscheme=<?php echo $fb_colorscheme ?>&show_faces=<?php echo $fb_show_faces ?>&bordercolor&stream=<?php echo $fb_stream ?>&header=<?php echo $fb_header ?>&height=<?php echo $fb_height ?>"
  10 scrolling="<?php echo $fb_scrolling ?>"
  11 frameborder="0"
  12 style="border:none; overflow:hidden; width:<?php echo $facebook_width ?>px; height:<?php echo $fb_height ?>px;"
  13 allowTransparency="true">
  14 </iframe>

Looking backwards, neither fb_likebox_theme() nor fb_likebox_block() sanitize variables like $fb_url, $fb_width, etc. I do notice that these text-input variables are controlled with validation in fb_likebox.admin.inc - but what happens when validation goes wrong? This is now open to security issues. Please see http://drupal.org/node/28984 and the section near the bottom on 'In practice'.

Looks good to me, probable some one else will double check

I only had time to look through the code now, I will do a functionality review hopefully tomorrow.

Please apply the patch (see http://drupal.org/node/52287 for commit message conventions), and set back to "needs review". Leave it assigned to me, so I don't lose track.

Attachment failed.

Another patch with some code correction and better use of the t() function.

Left as an exercise: You could move all your settings inside hook_block, it will make it easier for end-users to configure.

Where in the coding standards does it say to have a LICENSE.txt? Unfortunately, that is incorrect, and you need to remove it.
Once you do that, I believe you are pretty much ready to go.

Sorry for another delay, but you are almost done!

The LICENSE.txt file is automatically added by the drupal.org packaging script when it makes the tgz/zip downloads, as well as adding the version information to the .info file.

I've granted you full project creation rights. Use this ability carefully!

Thank you for your patience.