jbiechele [jbiechele]

1. The file LICENSE.txt needs to be removed; Drupal.org CVS doesn't allow to commit that file.

2. function is24_menu() {
     $items = array();
     $items['admin/settings/is24'] = array(
       'title' => 'ImmobilienScout24.de',
       'access arguments' => array('administer is24'),
       'page callback' => 'drupal_get_form',
       'page arguments' => array('is24_settings_form'),
       'file' => 'is24.admin.inc',
       'type' => MENU_NORMAL_ITEM,
     );
     
     return $items;
   }
   
   
   /**
    * Implement hook_perm()
    */
   function is24_perm() {
     return array('administer ImmobilienScout24.de');
   }
   

   The defined permission is not the used one.

3.     case 'list':
         $blocks[0] = array(
           'info' => t('ImmobilienScout24.de Estate Offers'),
         );
   

   Strings used in user interface should have the first word in capital case, and the other words in lower case (with the exception of proper nouns, adjectives derived from proper nouns, and acronyms).

4.             '#options' => array(
               	'AddressCity' => 'Address City', 
               	'CreationDate' => 'Creation Date', 
               	'DeactivationDate' => 'Deactivation Date', 
               	'Distance' => 'Distance', 
               	'ForignKey' => 'Foreign Key', 
               	'GroupNumber' => 'Group Number', 
               	'Heading' => 'Heading', 
               	'ImmoType' => 'Estate Type', 
               	'LastModificationDate' => 'Last Modification Date', 
               	'MarketingPrice' => 'Marketing Price', 
               	'NetArea' => 'Net Area', 
               	'NetRent' => 'Net Rent', 
               	'NoRooms' => 'No Rooms', 
   

   Strings used in the user interface needs to be translated; this includes also the strings used as options for the form fields.

5. function is24_install() {  
     variable_set('is24_api_key', '');
     // Set is24.de test vendor id
     variable_set('is24_vendor_id', '6687764');
     variable_set('is24_block_offers_items', '3');
     variable_set('is24_block_offers_type', array('all' => 'all'));
     variable_set('is24_block_offers_image_type', 'Expose');
     variable_set('is24_block_offers_sort_criteria', 'LastModificationDate');
     variable_set('is24_block_offers_sort_order', 'DESC');
     variable_set('is24_block_offers_show_heading', '1');
     variable_set('is24_block_offers_show_image', '1');
   }
   

   Drupal variables should not be initialized to their default values during installation.

@jbiechele: There no reason to feel sorry. It can happen that what I report is not clear; anyway, this is the first review for your code.

1. /**
   * Implementation of hook_enable().
   */
   function is24_enable() {
     drupal_set_message(t('ImmobilienScout24.de API bridge successfully installed. <br />1. Specify your vendor ID and API key in the <a href="@settings_config">configuration settings</a>.  <br />2. Enable a block and configure the display options in <a href="@settings_block">block settings</a>.',
       array(
         '@settings_config' => url('admin/settings/is24'),
         '@settings_block' => url('admin/build/block/list'),
       )));
   }
   

   That hook is called all times the module is enable, not only when the module is installed. The message could confuse users who would wonder why the module keeps to install itself. If there is the need to show such message, the code can be moved in hook_install().

2. /**
   * Implement hook_uninstall()
   */
   function is24_uninstall() {  
     // Delete is24 variables
     $result = db_query("SELECT name FROM {variable} WHERE name LIKE 'is24_%'");
     while ($row = db_fetch_object($result)) {
       variable_del($row->name);
     }
     // Delete is24 block
     db_query("DELETE FROM {blocks} WHERE module = 'is24'");
   }
   

   It is better to call variable_del() for each variable the module defines. This avoids any conflict with other modules that could have a name starting with is24_.

3.             '#multiple'   => 'TRUE',
   

   The value should be a boolean, not a string. It works simply because any not empty string would have the same effect of the boolean TRUE, but that is also true for the string FALSE.

I apologize for the delay in 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.