Dog Weather

Manual review

1. Use 6.x-1.x branch instead of master for development. Check Release naming conventions
2. use a package name in your .info file to keep separation of your module with others
3. use check_plain function while retrieve and storing data from xml
4. its best you use an template file if there is lot of html and styling required instead of writing everything in the theme function.
5. in some of your sql queries you have used %d for the placeholder zipcode in single quotes and some places you did not. Please maintain consistency.

There were many coding standard issues with this code.

Please see the attached file.

Also read these and follow the coding standards.

http://drupal.org/coding-standards
http://drupal.org/node/1354

Hi,

nice module, a few suggestions though

• instead of a long list of INSERT's in _dog_weather_fill_icao_table(), the data could be in a separate CSV file, this will reduce the size of the module (and perhaps use drupal_write_record instead of db_query("INSERT...")
• is there a reason for _dog_weather_truncate (is round not precise enough ?)

In addition:

• you may want to use t() in the tpl.php files, to allow the labels like Day, Night, High, Low... to be translated (e.g. in Spanish)
• is the module limited to US cities ? if it is not limited, have you checked what happens in dog_weather_forecast_page(), $pos = strrpos($account->state, "-"); when a non-US country (without two-character State) is selected, e.g. France Nice ?
• if it is limited to US cities, then why are the coordinates of all the other stations added to the database?
• most non-US citizens use degree Celsius instead of Fahrenheit, and kilometer per hour instead of miles per hour, so it would be nice to make these units available as an option
• url's in _dog_weather_get_current_observations() and _dog_weather_get_latitude_longitude() are hard-coded, would be nice to be able to change them in an admin page

Sorry for the delay! :(
We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

Review of the 6.x-1.x branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

manual review:

1. project page is too short, see http://drupal.org/node/997024
2. dog_weather_schema(): why do you have to store data in Fahrenheit and Celsius? Couldn't you just store one value and calculate the other?
3. "'access arguments' => user_access('access weather forecast'),": wrong usage of the hook_menu() API, you should just pass the permission string in an array.
4. "GLOBAL $user;": the global keyword should be all lower case.
5. "/* ELIPSE RADIUS ON X AXIS */": inline comments should use "//" comment style and should be proper sentences with standard capitalization. See http://drupal.org/node/1354#inline
6. "'#title' => 'City, State or Zip',": all user facing text must run through t() for translation, please check all your strings.

Review of the 6.x-1.x branch:

• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. dog_weather_init(): so your module depends on the dogzville theme? Please add that requirement to the README file or move the CSS into the module.
2. dog_weather_forecast_page(): the permission 'administrator users' does not exist, do you mean 'administer users'?
3. dog_weather_forecast_page(): where does $user->dogzville['owner'] come from? Same for $account->zipcode.
4. "variable_get('dog_weather_observations_url', 'http://www.nws.noaa.gov/xml/current_obs/all_xml.zip');": if your module provides its own variables you should remove them in hook_uninstall().
5. "'src' => '/files/dogzville/images/button_go.png',": remove all hard coded paths that are not available on other drupal installations.
6. Make sure to review more other project applications to get a review bonus as strongly recommended in the application documentation.

The field icao in the table dog_weather_icao is assigned as Autoincrement primary key. You should not give a value to that field in the insert queries. MySQL will give the value automatically.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

Working on review bonus.