himue [himue]

Hello, and thanks for applying for a CVS account. I am adding the review tags, and some volunteers will review your code, reporting what needs to be changed.

I would suggest to expand the description of the proposed project, and report the differences between it and the existing projects. The
other modules are too complicated, and I created a simpler module reason is not sufficient to add a project that duplicates the work done in other projects (see http://drupal.org/node/539608).

1. Schema descriptions should not be passed to t(); see what Drupal core modules do.

2. require_once("himuesgallery.php");
   

   If the code is always included, then it is better to merge the files. There is not pro to separate two files, if then one is always included. The purpose to write code in separated files is to reduce the memory usage, but that doesn't happen in this case, where there is also a delay caused by loading a file.

3.   //Wir geben ein Array zurück, weil ein Modul mehrere Node-Typen definieren kann.
     //Wir definieren nur einen Node-Typ 'himuesgallery'
   

   Code comments should be written in English.

4.   $form['meine'] = array(
       '#title' => t('Gallerie Settings'),
       '#type' => 'fieldset',
       '#collapsible' => TRUE,
       '#weight' => -10
     );
   

   Strings used in the user interface should be in sentence case, not title case.

5.   $form['meine']['thumbnail_sort_order'] = array(
       '#type' => 'radios',
       '#required' => TRUE,
       '#title' => t('Sortorder for thumbnails (0=asc, 1=desc)'),
       '#options' => array(
         t('asc'),
         t('desc')
       ),
       '#default_value' => isset($node->thumbnail_sort_order) ? $node->thumbnail_sort_order : 0,
       '#weight' => -1,
     );
   

   meine should be main. Then the form item should be simply write as

     $form['thumbnail_sort_order'] = array(
       '#type' => 'radios',
       '#required' => TRUE,
       '#title' => t('Sortorder for thumbnails (0=asc, 1=desc)'),
       '#options' => array(
         t('asc'),
         t('desc')
       ),
       '#default_value' => isset($node->thumbnail_sort_order) ? $node->thumbnail_sort_order : 0,
       '#weight' => -1,
     );
   

   It's useless to group form fields when there is just a group (the meine group).

6. function _himuesgallery_convert_description($description) {
     $description=str_replace(".br.", "<br>", $description);                  //break
     $description=str_replace(".-i.", "</i>", $description);                  //italic off
     $description=str_replace(".i.", "<i>", $description);                    //italic on
     $description=str_replace(".-b.", "</b>", $description);                  //bold off
     $description=str_replace(".b.", "<b>", $description);                    //bold on
     $description=str_replace(".ap.", "'", $description);                     //apostroph
     $description=str_replace(".es.", "'", $description);                     //apostroph
     $description=str_replace(".af.", '"', $description);                     //question mark
     $description=str_replace(".qm.", '"', $description);                     //question mark
     $description=str_replace(".bs.", "\"", $description);                    //backslash
     $description=str_replace(".-red.", "</font>", $description);             //red text off
     $description=str_replace(".red.", "<font color=red>", $description);     //red text on
     $description=str_replace(".-blue.", "</font>", $description);            //blue text off
     $description=str_replace(".blue.", "<font color=blue>", $description);   //blue text on
     $description=str_replace(".-green.", "</font>", $description);           //green text off
     $description=str_replace(".green.", "<font color=green>", $description); //green text on
     $description=str_replace("_", " ", $description);                        //replace underline with space
     $description=str_replace(".qe.", '?', $description);                     //question mark
     $description=str_replace(".fz.", '?', $description);                     //question mark
     $description=str_replace(".-right.", "</div>", $description);            //right-align text off
     $description=str_replace(".right.", "<div align=right>", $description);  //right-align text on
     $description=str_replace(".-left.", "</div>", $description);             //left-align text off
     $description=str_replace(".left.", "<div align=left>", $description);    //left-align text on
     $description=str_replace(".center.", "", $description);     
   

   All that code can be replaced with a single call to str_replace().

7.   $Seitenverhaeltnis=10/15;
     $UmgekehrtesSeitenverhaeltnis=15/10;
   

   Also variables should use English words.

8.     while ($file = @readdir ($handle))
       {
         //Test, if this is one of the supported filetypes. Otherwise ignore the file.
         $extension=substr($file,-4);
         if ((strtoupper($extension) == ".JPG") or (strtoupper($extension) == ".PNG") or (strtoupper($extension) == ".GIF")) {
           $files[$i]['File']=$file;
           
           //If first three charachters are an integer and fourth character is an "-" then dont show these charachters
           //So you can sort the files in any aorder you want if you number it
           if (((intval(substr($file,0,3)))<>0) AND ($file[3]=="-")) {
             $files[$i]['description'] = substr($file,4,-4);
           }
           //if no leading integer, the first 4 charachters are member of the filename.
           else {
             $files[$i]['description'] = substr($file,0,-4);
           }
   

   There is a Drupal function to scan a directory content. The code should use the Drupal Unicode functions, when available.

9.     if ($node->thumbnail_sort_order<>0) {
         @rsort($files);
       }
       else {
         @sort($files);
       }
   

   There is no reason to suppress error warnings, in that case; the function doesn't emit any warning message.

10. See http://drupal/coding-standards to understand how a module should be written; in particular, see how could should be formatted, and how constants should be written.

11. <?php
    // $Id$
    /**
     * @file
     * Callbacks of thes administration site for himues gallery 
     */
     
    /**
     * Form creation
     *
     * @incggroup forms
     * @see system_settings_form().
     */
    function himuesgallery_admin_settigs() {
      $files_dir=file_directory_path();
      
      $form['himuesgallery_admin_settigs'] = array(
        '#type' => 'textfield',
        '#title' => t('The base directory of the gallerys for this site relativ to "%files_dir"', array('%files_dir' => $files_dir)), 
        '#default_value' => variable_get('himuesgallery_admin_settigs', ('himuesgallery')),
      );
      
      return system_settings_form($form);
    }
    

    There is no reason to create a file for that code; it can stay on the module file.

I think the group is'nt useless, because i found no other way to prevent, that "Menu settings" will between the body-field "Info-text in top of the gallery" and the rest of my settings.

To accomplish that is enough to set the weight of the fields, or add all the settings in a fieldset.

package = Himues Developement

The package name is not supposed to be used to report who developed the module; as this is just a single module, and not a set of modules, the package ID used should be one already used by other projects (User interface, in example), or it should not reported.

variable_get('himuesgallery_admin_settigs', ('himuesgallery'))

The parentheses surrounding the string are not necessary.

  t('You have to select a css file'));

css should be written in upper case letters.

  //read the subdirs under the gallery-standard-path wich is set in administration
  $himuesgallery_path = file_directory_path() . "/" . variable_get('himuesgallery_admin_settigs', '');
  $array_of_dirs = glob($himuesgallery_path . '/*', GLOB_ONLYDIR);

There is a Drupal function to get the content of a directory.

/**
 * delete the thumbnails
 */
function _himuesgallery_delete_thumbnails($path) {
  if (is_dir($path)) {
    $handle = opendir($path);
    
    while ($file = readdir ($handle)) {
      $prefix=substr($file, 0,6);
      $prefix=strtoupper($prefix);
      if ($prefix=="THUMB.") unlink($path."/".$file);
    }
    closedir($handle);
    drupal_set_message(t("himuesgallery: Thumbnails renewed."));
  }
}

There is a Drupal function to scan the content of a directory. The code should also use the Drupal Unicode functions.

See http://drupal/coding-standards, which reports how PHP constants should be written, and how the code should be formatted.

if ($dir->name == "") $array_of_dirs[] = $dir->filename;

The curly brackets are not considered optional from Drupal coding standards. The code is correctly written as

if ($dir->name == "") {
  $array_of_dirs[] = $dir->filename;
}

Before, and after the operator = there should be a space ($variable = 'value'); that is true also for the other operators.

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.