Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated internet_weather.pl to be compatible with new WeatherNOAA #637

Merged
merged 1 commit into from
Oct 24, 2016

Conversation

marcmerlin
Copy link
Collaborator

Also fixed $city -> -city "$config_parms{city}", not too sure how
it was working before, but it's not anymore.

Also fixed $city -> -city "$config_parms{city}", not too sure how
it was working before, but it's not anymore.
@marcmerlin marcmerlin merged commit 390eadf into hollie:master Oct 24, 2016
@marcmerlin marcmerlin deleted the fix_internet_weather branch October 24, 2016 04:26
@peloy
Copy link
Collaborator

peloy commented Oct 25, 2016

Hi Marc,

On Sun, Oct 23, 2016 at 09:07:04PM -0700, Marc MERLIN wrote:

Also fixed $city -> -city "$config_parms{city}", not too sure how
it was working before, but it's not anymore.
You can view, comment on, or merge this pull request online at:

#637

-- Commit Summary --

  • updated internet_weather.pl to be compatible with new WeatherNOAA

I also run into the same problem last night after pulling in the
WeatherNOAA changes. I am also scratching my head regarding how this was
working before.

The problem that you fixed was caused by the following code in
code/common/internet_weather.pl:

my $city = $config_parms{city};
$city = $config_parms{nws_city} if defined $config_parms{nws_city};

So $city is initialized with $config_parms{city} but then gets clobbered
by $config_parms{nws_city} because $config_parms{nws_city} exists/is
defined as it is created via the following line in the stock bin/mh.ini:

nws_city=

Perhaps some recent change caused things like "nws_city=" to create a
%config_parms element that is the empty string?

Given that $config_parms{nws_city} is defined (it's an empty string, but
defined nonetheless), I think the code in internet_weather.pl should
have been:

$city = $config_parms{nws_city} if defined($config_parms{nws_city}) && $config_parms{nws_city};

But... nws_city is only used in code/common/internet_weather.pl in
all of the MH code so I think the best thing to do is to get rid of it
altogether and just stick with "city" ($config_parms{city}).

I'll see about submitting a pull request for a small cleanup.

Cheers,

Eloy Paris.-

@marcmerlin
Copy link
Collaborator Author

So yeah, I was scratching my head too but didn't look as deeply as you.
If nws_city is used nowhere else, I'm totally cool with a CL just removing it altogether.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants