Wunderground API

Sorry, I should have been clearer in the other issue. While you've renamed the project, your module name in code is still the same as the other non-API wunderground module, which can cause big problems. If the current "wunderground" module ever gets a D7 release, and someone installs both on the same site, the site will have two functions both named wunderground_block_info(). This will cause a fatal error in PHP, crashing the whole site. So in addition to renaming the project, you should rename the module in code, e.g. change wunderground_block_info() to wunderground_api_block_info(), change wunderground.module to wunderground_api.modue, etc. Make sense?

it is not the right way to add css in the template file.
Proper way is to add your css in your hook_block_view() with drupal_add_css() or you create a template_preprocess_TEMPLATENAME() and then add the css via drupal_add_css().

PAReview.sh just find that

function  _wunderground_get_countries() 

is not right prefixed but in my eyes its okay ;-)

I cannot checkout using
git clone http://git.drupal.org/sandbox/alpixel/1781982.git wunderground_api

warning: remote HEAD refers to nonexistent ref, unable to checkout.

I read from http://drupal.org/node/1074584 that it could be caused by an unexisting master branch.

I would like to test the module... could you solve this ?

wuups sorry my hint with the css was a better way than yours but there is a real drupal way ;-)
The block is a render element so you can just attache the css file. So the css file would be cached with its block and it would be considerably easier to interact with hook_page_alter().

you can find more information at http://api.drupal.org/api/drupal/includes%21common.inc/function/drupal_r...
and a deeper look to drupal_process_attached()

unfortunately there is no example in the example_modules or i can't find it.

All the variables set in wunderground_api_settings_admin_form() need to be deleted with hook_uninstall(), which you should put in a new wunderground_api.install file.

i see that you attached taxonomy.js. Do you really need it? And the css file is still not attached as css file :p

attached like

$block['content']['#attached']['css'][] = drupal_get_path('module', 'mymodule') . '/mymodule.css';

Hi, i had some manual review for your module.
1. Fatal error: Call to undefined function _wunderground_get_countries() in Z:\home\card_maker\www\sites\all\modules\1781982-cdf639e\wunderground_api.admin.inc on line 84
You need to rename wunderground_get_countries() function to wunderground_api_get_countries()
on the line 84 and 103 in wunderground_api.admin.inc.

2. You can place module settings on block configure page. See hook_block_configure().

3. Add t() for almost all #options in wunderground_api.admin.inc. And in the function wunderground_api_get_countries() too.
Use t() for the watchdog messages in WundergroundAPICall.php.

4. In "wunderground_block.tpl.php" on the line 72 use t() for "km/h".
You need to set measurement units from configure page in "wunderground_block.tpl.php" instead of fixed "°C" or "km/h".

5. And automated review said this

FILE: ...sites/all/modules/pareview_temp/test_candidate/wunderground_api.install
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
8 | ERROR | Function comment short description must start with a capital
| | letter
8 | ERROR | Function comment short description must end with a full stop
11 | ERROR | Opening brace should be on the same line as the declaration
--------------------------------------------------------------------------------

Lovely module! A few notes:

• Please include the actual git command in your issue summary, ie.
  

  git clone http://git.drupal.org/sandbox/alpixel/1781982.git wunderground_api

• You still need to delete the master branch. See http://drupal.org/node/1659588
• When you renamed the module, you missed one of the functions. In wunderground_api.admin.inc on line 103, wunderground_get_countries() should be wunderground_api_get_countries()
• Thank you for adding comments to your code, but please update wunderground_api.install to comply with Doxygen comment standards. For more information: http://drupal.org/node/1354
• Thank you for wrapping your text to 80 columns. However, you went slightly over on line 72 of wunderground_block.tpl.php, and it's causing problems with your concatenation symbol. (The code might still work, but Ventral doesn't like it; see below.)

Also, please check the automated review at http://ventral.org/pareview/httpgitdrupalorgsandboxalpixel1781982git before you request another "human" review.

Hello,

I enjoyed trying out this module. This is my manual review, and it's more of a usability review than a code review.

1) I had issues with getting the code via Git. My output:

% git clone http://git.drupal.org/sandbox/alpixel/1781982.git wunderground_api
Cloning into wunderground_api...
remote: Counting objects: 124, done.
remote: Compressing objects: 100% (120/120), done.
remote: Total 124 (delta 73), reused 0 (delta 0)
Receiving objects: 100% (124/124), 30.11 KiB, done.
Resolving deltas: 100% (73/73), done.
warning: remote HEAD refers to nonexistent ref, unable to checkout.

I was able to get past this by doing:

% git checkout 7.x-1.x
Branch 7.x-1.x set up to track remote branch 7.x-1.x from origin.
Switched to a new branch '7.x-1.x'

I wonder if there is still some Git manipulation on the remote that needs to be done.

2) Files with the Execute Bit

All your files come down from the repository with the execute bit set. Is this necessary? This is not something I've noticed with other modules.

3) English Measurements are Good

I like how the configuration screen has English as well as Metric measurements.

4) Configuration Doesn't Account for the States of the US

The location section only asks for City and Country. I'm from the United States, so I tried specifying Boston and United States. When I did that, the block didn't work. It produced lots of errors like this:

Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 79 of /home/rumali/dgd7/web/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 79 of /home/rumali/dgd7/web/sites/all/modules/wunderground_api/WundergroundAPICall.php).

When I make the web service call (see next item), I observe that the JSON output is a list of all the US states with Boston as a city. There is no current_observation object in the JSON. (See attached JSON.)

When I changed the configuration to Paris (city) and France (country), the wunderground_block.tpl.php worked properly!

5) Use of Watchdog is Excellent

I like that your module uses Watchdog. That enabled me to see the exact web service calls that I made:

http://api.wunderground.com/api/APIKEY/forecast/lang:EN/q/US/Boston.json
http://api.wunderground.com/api/APIKEY/conditions/lang:EN/q/US/Boston.json

Please let me know if there is an issue on my side. If you can enable the configuration to accept US states, it would open up the module for a wider audience.

Good luck with the process!

updating the git

Fast follow-up: The Wunderground API allows for US states as part of the GET request like so:

http://api.wunderground.com/api/APIKEY/conditions/lang:EN/q/US/MA/Boston...

Note the "MA" before Boston.json. This output contains the attached JSON (with current_observations).

#18 - When this module is made usable in the USA, please consider making the tempurature & wind speed as configurable options. Right now they are hard coded in the tpl for Celsius and km/h. Or maybe this isn't related to usability in the USA and is a bug?!? I noticed in WundergroundAPICall.php you provide the means to use either Celsius/Fahrenheit or km/miles.

WundergroundAPICall.php

• Line 31: Text should probably read Wunderground has not been configured, etc.
• Line 32: You could handle all this through hook_requirements(). See http://api.drupal.org/api/drupal/modules%21system%21system.api.php/funct...
• Line 40: You check for $request->status == 200, etc. to give meaningful error messages for 4xx, 5xx?

wunderground_api.admin.inc

• Line 20: Longer than 80 characters.
• Line 37: 12 hours isn't always the default if you change it, this is unclear to me.
• General comment: maybe add the configuration of city, country, units in hook_requirement?
• wunderground_api_get_countries - if certain countries are known not to work initially, you should remove them i.e USA.

Hello

Then I set up the block region and save I get a lot of notice:

Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 78 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 78 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 79 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 79 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 80 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 80 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 82 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 82 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 83 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 83 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 85 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 85 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 92 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 92 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 93 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 93 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 100 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 100 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Undefined property: stdClass::$current_observation in WundergroundAPICall->getCurrentWeather() (line 101 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).
Notice: Trying to get property of non-object in WundergroundAPICall->getCurrentWeather() (line 101 of /home/vladimir/lamp/apache2/htdocs/drupal7.local/sites/all/modules/wunderground_api/WundergroundAPICall.php).

See attached screenshot.

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

I'm a robot and this is an automated message from Project Applications Scraper.

Hello. I get the same error messages as mentioned in #20.
Any fix or news on that?

Cheers, Moritz

Is anyone still working on this? It would have been nice to make this one as a "fork?" of the previous version 6 Weather Underground module, even if that one wasn't using the official API. I'd be happy to help if I can.

I'm looking at the code, don't know if I can achieve anything but I hope I can and I hope I'm not treading on any toes.

Do you have the Wunderground API key from Weather Underground? Thanks. Will.

I'm just trying to get my host upgraded to the required version of PHP, so I can test and see what issues I get with the module.

Regarding #20, it seems that for some reason the call isn't returning the expected "current_observation". I'm not yet sure how to capture the request or the resulting JSON file to check for either what is returned in terms or data or error.

This issue is an application for full project access for alpixel, and was closed when alpixel stopped responding. pazza98, it looks like you're using this issue as a general discussion thread for your own work on the module, which isn't really what this is for. The project has its own issue queue:

https://drupal.org/sandbox/alpixel/1781982

Please continue any discussion not related to alpixel's full project access there. If alpixel does not return, you might also consider following the abandoned project process: https://drupal.org/node/251466

Hi sreynen,

Thanks for letting me know. I wasn't sure what the process was, so appreciate the polite pointer and apologise for not reading all the documentation and therefore also for not following the processes properly.

Kind Regards,

Will.

git info