kerberos [kerberos]

@kerberos: We just review a module / theme per applicant.

There is already a module with such functionality: http://drupal.org/project/appbar.

1. /**
    * Implementation of hook_theme().
    *
    * Returns information about every themable function defined by the module.
    */
   
   function theme_link_bar($links) {
     $output = '
      <div class="button" onclick="toggle();">X</div>
     <div id="bottomlinksbar">'. implode('&nbsp;', $links) .'</div>
     ';
     
     return $output;
   }
   function linkbar_theme() {
     $items = array();
     $items['linkbar_overview_form'] = array(
       'arguments' => array('form' => array()),
       'file' => 'linkbar.admin.inc',
     );
     $items['linkbar_record'] = array(
       'arguments' => array('record' => array()),
       'file' => 'linkbar.pages.inc',
     );
     $items['link_bar'] = array(
         'arguments' => array('links' => NULL),
       );
     
     return $items;
   }
   

   The comment refers to the second function, not the one directly below it. It would also be better to separate the functions with an empty line.

2.   drupal_add_css(drupal_get_path('module', 'linkbar') . '/styles.css');
     drupal_add_js(drupal_get_path('module', 'linkbar') . '/js.js');
         
         if (($cache = cache_get('link_bar')) && !empty($cache->data)) {
           $output = $cache->data;
         }
         else {
     $links1 = db_query("SELECT link,content,title from {linkbar_record} ORDER BY weight"); 
     while ($row = db_fetch_array($links1)) {
     $linkuri[]= l(  '<div class="linkbar_icon"></div><span>' . $row['title'] . '</span>', $row['link'], $options = array('attributes' => array('target' => "_blank", 'class' => $row['content'] .' linkbar_link', 'title' => $row['title']), 'html' => TRUE));
     }
           $output = theme('link_bar', $linkuri);
           cache_set('link_bar', $output);
         }
     $block['content'] = $output;
   

   The indentation of the code is not correct; check also that you are using two spaces, and not a tab character.

3.     'description' => t('Stores custom links to be added to nodes.'),
       'fields' => array(
         'record_id' => array(
   

   Schema descriptions should not be passed to t() anymore. See system_schema() for an example of what done by Drupal core code.

4. function linkbar_update_6102() {
     $new_column = array(
           'type' => 'text',
           'not null' => FALSE,
            'size' => 'big',
           'description' => t('A description of the term.'),
         );
   
     $ret = array();
     db_add_field($ret, 'linkbar_record', 'link', $new_column);
     return $ret;
   }
   

   The update function is not executed when the module is first installed. As the module is not still hosted on drupal.org, you can remove that function.

5. /**
    * Implementation of hook_uninstall().
    *
    * This hook is called when the already-disabled module is explicitly uninstalled
    * by the administrator -- simple disabling the module will trigger hook_disable().
    * It should delete any database tables added by the module, remove any variables
    * that are unique to the module, and clear out any cached data.
    */
   

   The comment should say Implements hook_uninstall() (as other hooks as well).

There have not been replies from the OP in the past 7 days. I am marking this report as won't fix.

Remember to change the status, when you upload new code.

1.   $schema['linkbar_record'] = array(
       'description' => t('Stores custom links to be added to nodes.'),
       'fields' => array(
         'record_id' => array(
           'type' => 'serial',
           'unsigned' => TRUE,
           'not null' => TRUE,
           'description' => t('Unique identifier for the {linkbar_record}.'),
         ),
   

   Database schema descriptions should not be passed to t(); see what done from Drupal core modules.

2. See http://drupal.org/coding-standards to understand how a module should be written; in particular see the part about how the code should be formatted, and the character used as indentation.
3. As the code has not been hosted on Drupal.org repository, the update function should not exist.

4. function linkbar_record_load($record_id) {
     $sql = "SELECT * FROM {linkbar_record} WHERE record_id = %d";
     $result = db_query($sql, $record_id);
     if ($record = db_fetch_array($result)) {
       return $record;
     }
     else {
       return FALSE;
     }
   }
   

   As the function is just returning a record, the code should use db_query_range().

5. The comments for hook implementations should be like the following one

    /**
     * Implements hook_theme().
     */
   

Why the status changed to "fixed"? Was the applicant granted CVS permission?

Yes, he did.
I change the status to fixed right before to approve a CVS account.

Ok thanks, I just wanted to verify, as he hasn't any project or commit.

Please don't use the CVS application issue to report issues with existing modules. The purpose of this queue is to document the process of approving a CVS account.

Set the comments to read only.