digitalsweetspot [digitalsweetspot]

• 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. The version line needs to be removed from the .info file.

2. /**
    * Implementation of hook_uninstall().
    */
   function status_animate_uninstall() {
     db_query("DELETE FROM {variable} WHERE name LIKE 'status_animate'");
   }
   

   Hook implementation comments should be like the following one:

   /**
    * Implements hook_menu().
    */
   

   Deleting Drupal variables using a query that matches any Drupal variable with a name that starts with the module name would remove also the Drupal variables of other modules.
   In this case it is then enough to use variable_del('status_animate').

3. /**
    * Implementation of hook_install().
    */
   function status_animate_install() {
   	$statusAnimate = array(
   		'style' => 'fadeOut',
   		'duration' => 'slow',
   		'delay' => 2000,
   		'pages' => '',
   	);
   	
   	variable_set('status_animate', serialize($statusAnimate));
   }
   
   

   It would be probably better to use more Drupal variables, and avoid to use that code.

4. function status_animate_help($path, $arg) {
     switch ($path) {
       case 'admin/help#status_animate':
         $output = t("<p>The Status Animate module provides a transitional effect on status messages, so that they will disappear after a pre-determined time interval.</p>
   				<p>Status Animate offers a full <a href=\"!url\">admin panel</a> where administrators can select from the following settings:</p>
   				<ul>
   					<li><strong>Animation style</strong> (the type of transition)</li>
   					<li><strong>Animation duration</strong> (the amount of time before the transition occurs)</li>
   					<li><strong>Animation delay</strong> (the amount of time the transition takes to complete)</li>
   					<li><strong>Page specific visibility settings</strong> (list of pages where the transition will not occur)</li>
   				</ul>
   				<p>It is important to note that this module only affects \"status\" messages, and not \"error\" messages or \"warning\" messages.</p> 
   				<p><a href=\"!url\">Click here to administer Status Animate.</a></p>
   				<p></p>
         ", array('!url' => url('admin/user/status_animate')));
         return $output;
         break;
     }
   }
   
   

   Avoid to escape a string delimiter inside a string, especially if it is passed to t().
   The correct placeholder for URLs doesn't start with !.

5. function status_animate_page() {
   	$path = base_path() . drupal_get_path('module', 'status_animate');
   	$output .= drupal_get_form('status_animate_settings_form');
   	return $output;
   }
   
   

   The variable $path is not used from the function.
   The variable $output is initialized once; there is no need to use the operator .=. There would no need to use the variable $output, if the function would use return drupal_get_form('status_animate_settings_form');.
   Using a better definition of the menu callback, there would not be the need to use two functions for a menu callback.

6. 	$dbData = variable_get('status_animate', '');
   	$statusAnimate = unserialize($dbData);
   
   

   There is no need to serialize/deserialize a Drupal variable; that is done from variable_set()/variable_get().

7. function status_animate_settings_form_submit($form, &$form_state) {
   	
   	$statusAnimate = array(
   		'style' => $form_state['values']['style'],
   		'duration' => $form_state['values']['duration'],
   		'delay' => $form_state['values']['delay'],
   		'pages' => $form_state['values']['pages'],
   	);
   	
   	variable_set('status_animate', serialize($statusAnimate));
   	drupal_set_message('Settings saved');
   }
   
   

   All that code would not be needed, if the module would use more than one Drupal variable, and it would use sytem_settings_form().

8. If I correctly recall, there is already a module with the same purpose.

Hi. Please read all the following and the links provided as this is very important information about your CVS Application:

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:
Migrating from CVS Applications to (Git) Full Project Applications

• The status of this application will be put to "postponed" and by following the instructions in the above link, you will be able to reopen it.
• Or if your application has been "needs work" for more than 5 weeks, your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.