tungt84 [tungt84]

Hello, and thanks for applying for a CVS account.

As per Apply for contributions CVS access, the motivation should include details about the module that you intend to add to Drupal.org repository, and explain the differences between it, and any existing projects.
The archive containing the code needs to be uploaded here, in a comment.

There is already a module that shows traffic rank as reported by Alexa: http://drupal.org/project/sitestats.

I am changing the status.

I am changing status so users will review the project.

1. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how the code should be formatted, and how constant names should be written.

2. 	drupal_install_schema('alexaoption');
    $defaul_site='box-idea.com';	
    $res = db_query(
       "SELECT vid FROM {alexaoption} LIMIT 1"
     );
     $has_format = db_result($res);
     
     // Create format
     if (!$has_format) {
       db_query(
         "INSERT INTO {alexaoption} (vid,nid,typewidget,siteurl1,siteurl2,siteurl3,width,height,optiontype,optionrange,bgcolor) VALUES (%d,%d,%d,'%s','%s','%s',%d,%d,'%s','%s','%s')",
         1,1,1, 
         $defaul_site,
         null,null,
         null,null,
         '120 x 95',
         null,null
       );
     }
   

   As the module is being installed, that SQL query cannot return anything, as the module database table is empty.

3.   $items['admin/settings/alexamod'] = array(
       'title' => 'Setup Alexa Widget',
       'page callback' => 'alexamod_setting',
       'description'=>'Setup Alexa Widget',
       'access arguments' => array('administer users'),
       'type' => MENU_NORMAL_ITEM,
     );
   

   Strings used in the user interface should be in sentence case.

4.     'access arguments' => array('administer users'),
   

   The menu callbacks should probably use a different permission; a site ranking widget doesn't show anything about the users of a Drupal-powered site.

1. The constants are still written in lower case characters, which is not what the coding standards say (see http://drupal.org/coding-standards#naming).

2. function _alexa_get_widget_option() {
     $res = db_query("SELECT * FROM {alexaoption} LIMIT 1");
     return db_fetch_object($res);
   }
   

   There is a Drupal function that limit the number of the rows returned, and it is db_query_range(); the syntax used for the query is probably specific for a database engine (differently, the function db_query_range() would have not been created).

3. Coding standards don't say to not add an empty line left between a function, and the other; that empty helps to make the code more readable.

Hi

I've had a look at your module and there are a number of issues that need addressing.

1. Inconsistent function naming: some functions begin with "alexamod" and some with just "alexa" they should all be "alexamod". Also the module directory should be the same and should not have the hyphen.

2. Your menu callbacks are unnecessarily complicated, for example instead of:

  $items['admin/settings/alexamod/alexatrafficgraph'] = array (
    'title' => 'Setup alexa traffic graph widget',
    'page callback' => 'alexamod_alexatrafficgraph',
    'type' => MENU_CALLBACK,
  );

...

function alexamod_alexatrafficgraph() {
  return drupal_get_form('alexamod_alexatrafficgraph_form');
}

You can do this:

  $items['admin/settings/alexamod/alexatrafficgraph'] = array (
    'title' => 'Setup alexa traffic graph widget',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('alexamod_alexatrafficgraph_form'),
    'type' => MENU_CALLBACK,
  );

And you can lose the alexamod_alexatrafficgraph() function.

3. Having run your module through the "coder" module (http://drupal.org/project/coder) analysis:

a. Lots of places where there's no space after a comma;
b. Space between "array" and its following parenthesis;
c. "else" not starting on its own line;
d. TRUE, FALSE and NULL and SQL keywords need to be uppercase;
e. Many lines have trailing spaces;
f. @file block missing at the beginning of files;
g. HTML tags need to be lowercase;
h. PHP variables need to be all lowercase;

These issues are all described on the http://drupal.org/coding-standards pages.

Hope this helps.

I am changing the status as per previous comment.

There have not been replies in the last week. I am marking this application as won't fix.

This issue is not fixed until you get a CVS account.

• The points reported in this review are not in order or importance / relevance.
• Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
• Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.

1. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how functions, Drupal variables, global variables, and constants defined from the module should be named; how the code should be formatted; how constants should be written.

2. define(ALEXAMOD_PERM, 'administer alexamod');
   
   

   The first argument of define() is a string.

See my previous point #1, which is still valid.

I'd like to contribute some suggestions:

- In the zip package, your files should be placed in a directory.

- Inconsistent of like break between functions: Some functions have no line break between, while some have. See post #12 item 3.

- You should add comments for the implemented hooks such as hook_install(), hook_block() etc...

D7 like:

/**
* Implements hook_install().
*/

D6 like:

/**
* Implementation of hook_install().
*/

- While '3m' => t('3 months') using plural of "months", '3y' => t('3 year') should use plural of "years" too.

Please clean & prepare CVS header for each files (// $Id$ for .php / .install, ; $Id for .info, $Id$ for .txt files...).

This issue queue is for CVS applications; for the other question (for which I don't have a reply, as I didn't know that project was created) open an issue report in the Drupal.org webmasters queue.

I have posted discussion of issue to http://drupal.org/node/1034730

I am approving this application.

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, 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.

I thank all the dedicated reviewers as well.

I am giving credits to the users who participated in this issue.