11870.com module [#1335448]

Comment #1

raynimmo commented 9 November 2011 at 17:16

Status:

Needs review

» Needs work

There are quite a few issues you really have to deal with before you will have any chance of getting this module approved.

First an easy one, your .info file should be treated just like any other text file and doesnt need the opening php tags.

Also, the ;$Id$ descriptor will be added by Drupal's packing script and is not required.

Master Branch
It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

Installation
I have actually had trouble getting your module enabled with Drupal 7, I am not quite sure of the exact naming convention allowances with regards to numbers within the module name and file naming but the only way I could enable your module was by renaming the module folder and the module and install files.

3rd Party Code
There is a lot within your module that appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms. The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

The files in question are;
simplepie_1.2/simplepie.inc
- Copyright (c) 2004-2009, Ryan Parman and Geoffrey Sneddon, All rights reserved.

geoip.inc
- Copyright (C) 2007 MaxMind LLC - GNU LPG License

geoipcity.inc
- Copyright (C) 2004 Maxmind LLC - GNU LPG License

geoipregionvars.php
- Copyright 2009 Maxmind LLC All Rights Reserved

Code Standard
With regards to your actual module code, I was able to enable it within a version of Drupal 6 to perform testing. You should run the Coder module to check your style, some issues were found (please check the Drupal coding standards):

Coder settings: Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
11870_module.module

severity: minor Line 6: indent secondary line of comment one space

*/

severity: normal Line 12: Separate comments from comment syntax by a space.

//'status' => TRUE,

severity: normal Line 13: Separate comments from comment syntax by a space.

'region' => 'sidebar_first', //CONFIGURE ME!!

severity: normal Line 36: Separate comments from comment syntax by a space.
//'#default_value' => variable_get('onceocho_topic', 'arabe'),

severity: normal Line 106: Use "elseif" in place of "else if"

else if ($maxdisp <= 0) {

severity: normal Line 124: Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
$output = null;

severity: normal Line 134: use a space between the closing parenthesis and the open bracket

          if ($f){

severity: minor Line 135: Use an indent of 2 spaces, with no tabs

$gi = geoip_open($f, GEOIP_STANDARD);

severity: normal Line 138: Separate comments from comment syntax by a space.

if ($ip == "127.0.0.1") $ip = "217.71.192.13"; //WHATEVER

severity: normal Line 143: use a space between the closing parenthesis and the open bracket

if (!is_object($record)){

severity: normal Line 150: Separate comments from comment syntax by a space.

$limit = 7; //change me if you want to grab more items

severity: normal Line 162: Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE

return null;

severity: normal Line 165: Separate comments from comment syntax by a space.
//foreach ($feed->get_items() as $item) {

severity: normal Line 166: Use PHP's master function, not an alias. (List of PHP aliases)

$max = sizeof($feed->get_items());

severity: normal Line 170: Use the matching Drupal theme functions, not raw HTML.

$output.= "<table><tr><td>";

severity: normal Line 170: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output.= "<table><tr><td>";

severity: normal Line 178: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms
$output.= "" . $direcc[0]['data'] . ", " . $loc[0]['data'] . "";

severity: normal Line 179: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output.= "</td></tr><tr><td align='center'>";

severity: normal Line 180: Use the matching Drupal theme functions, not raw HTML.

$output.= "<a href='" . $item->get_link() . "'>" . $item->get_title() . "</a> ";

severity: normal Line 180: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output.= "<a href='" . $item->get_link() . "'>" . $item->get_title() . "</a> ";

severity: minor Line 180: use
instead of

$output.= "<a href='" . $item->get_link() . "'>" . $item->get_title() . "</a> ";

severity: normal Line 181: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms
$output.= "</td></tr>";

severity: normal Line 182: use a space between the closing parenthesis and the open bracket

if ($foto[0]['attribs']['']['href']){

severity: normal Line 183: Use the matching Drupal theme functions, not raw HTML.
$output .= "<tr><td align='center'><img src='".$foto[0]['attribs']['']['href']."'/></td></tr>";

severity: normal Line 183: Use an indent of 2 spaces, with no tabs
$output .= "<tr><td align='center'><img src='".$foto[0]['attribs']['']['href']."'/></td></tr>";

severity: normal Line 183: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output .= "<tr><td align='center'><img src='".$foto[0]['attribs']['']['href']."'/></td></tr>";

severity: normal Line 184: else statements should begin on a new line

}else{

severity: normal Line 185: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output.= "<tr><td align='center'> </td></tr>";

severity: normal Line 187: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output.= "<tr><td style='text-align: justify;'>";

severity: normal Line 189: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output.= "<cite>" . $item->get_description() . "<cite> ";

severity: minor Line 189: use
instead of

$output.= "<cite>" . $item->get_description() . "<cite> ";

severity: normal Line 192: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output.= "<cite>" . $tel[0]['data'] . "<cite> ";

severity: minor Line 192: use
instead of

$output.= "<cite>" . $tel[0]['data'] . "<cite> ";

severity: normal Line 194: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms
$output.= "</td></tr>";

severity: normal Line 196: Separate comments from comment syntax by a space.

//}

severity: normal Line 197: Use the matching Drupal theme functions, not raw HTML.

$output.= "<tr><td weigth='50'><a href='http://11870.com/pro/" . $slug . "/map' id='map'>ver en mapa</a>  ·  <a id='map' href='http://11870.com/pro/" . $slug . "?callback=send-sms&ref=nf'>mandar datos a tu móvil</a></td></tr>";

severity: normal Line 197: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output.= "<tr><td weigth='50'><a href='http://11870.com/pro/" . $slug . "/map' id='map'>ver en mapa</a>  ·  <a id='map' href='http://11870.com/pro/" . $slug . "?callback=send-sms&ref=nf'>mandar datos a tu móvil</a></td></tr>";

severity: normal Line 198: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

$output.= "</table></div>";

Manual Review

@file doc block is missing in the module file, see http://drupal.org/node/1354#files .
Comments - your functions are lacking in any form of commenting to describe what each function does. All functions and methods, whether meant to be private or public, should be documented. A function documentation block should immediately precede the declaration of the function itself. See the Doxygen and comment formatting conventions with regards to functions.
There is a lot of code in there that really doesnt conform to modern web standards, have a read of the Drupal Markup Style Guide with particular attention to the use of tables and heavily nested tags.

Dont be disheartened as there really isnt that much in there to sort out.

Log in or register to post comments

Comment #2

leandro713 commented 24 November 2011 at 14:44

i have renamed the files, fixed minor issues and formatted my code with 'coder' module.
also commented functions, added @file doc block.
i will get rid of tables in next few days in a tpl with css

now i need a merciful soul drops me some enlightment about how to exactly integrate the 3rd party code.

Log in or register to post comments

Comment #3

jthorson commented 24 November 2011 at 15:45

leandro,

For integration of the 3rd party code, check out the 'Libraries API' documentation located at http://drupal.org/node/735160.

Hope this helps!

Log in or register to post comments

Comment #4

misc commented 3 February 2012 at 11:57

@leandro713 has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256