kubala.webdesign [kubalawebdesign]

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

As the other weather modules are not using the service provided by Google, the proposed module should be considered a duplicate of existing projects; if that would be the case, then any Ubercart payment modules should be considered duplicates of each other.

• 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 Drupal variables, global variables, constants, and functions defined from the module should be named; how the code should be formatted.

2.   db_query("DELETE FROM {blocks} WHERE module = 'menu_block'");
     db_query("DELETE FROM {blocks_roles} WHERE module = 'menu_block'");
     
     cache_clear_all();
   
   

   As far as I can see, no modules delete any reference to the blocks it implements from Drupal core tables.
   The call to cache_clear_all() is not necessary.

3. Hook implementation comments should be like the following one:

   /**
    * Implements hook_menu().
    */
   

4. function google_weather_install() {
     // No-op.
   }
   
   

   If a hook is not necessary, it is not defined.

5. Menu descriptions and titles, schema descriptions are not passed to t().

6.   $ch = @curl_init();
   	curl_setopt($ch, CURLOPT_URL, $url);
     curl_setopt($ch, CURLOPT_HEADER, 0);
     curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
     curl_setopt($ch, CURLOPT_TIMEOUT, variable_get('gw_timeout', 20));
     $data = @curl_exec($ch);
     $encoding = array_pop(explode('charset=', curl_getinfo($ch, CURLINFO_CONTENT_TYPE)));
     curl_close($ch);
   
   

   Why isn't the code using drupal_http_request()?

7. function google_weather_get_title($delta = 0) {
     $config = google_weather_get_config($delta);
     return t("Weather for ") . $config['gw_location'];
   }
   
   

   Use t()-placeholders, instead of concatenating strings.

8.     '#title' => t("How long forecast do you want?"),
   
   

   The title of a form field should not be a question.

• The points reported in this review are not in order of 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. License files cannot be committed in Drupal.org repository. Projects committed in Drupal.org repository have the same license used by Drupal.
2. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how Drupal variables, global variables, constants, and functions defined from the module should be named.

3.   $ch = @curl_init();
     curl_setopt($ch, CURLOPT_URL, $url);
     curl_setopt($ch, CURLOPT_HEADER, 0);
     curl_setopt($ch, CURLOPT_RETURNTRANSFER, TRUE);
     curl_setopt($ch, CURLOPT_TIMEOUT, variable_get('gw_timeout', 20));
   

   Why isn't the code using drupal_http_request()?

4. The module depends from a PHP5 function, but it doesn't declare its dependency from that version of PHP.

All the points I reported have been fixed.

Thank you for your contribution!
I am going to update your account so you can opt into security advisory coverage now.
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.