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

[weather][refactor] kill g_weatherManager global #13227

Merged
merged 6 commits into from Dec 22, 2017

Conversation

@xhaggi
Copy link
Member

commented Dec 21, 2017

see title

Copy link
Contributor

left a comment

nothing spotted, nice cleanup

@xhaggi

This comment has been minimized.

Copy link
Member Author

commented Dec 21, 2017

Ok thanks

@xhaggi xhaggi merged commit bc8b6d3 into xbmc:master Dec 22, 2017
1 check passed
1 check passed
default You're awesome. Have a cookie
Details
@xhaggi xhaggi deleted the xhaggi:kill-global-weathermanager branch Dec 22, 2017
@Rechi Rechi added the v18 Leia label Dec 22, 2017
@Rechi Rechi added this to the L 18.0-alpha1 milestone Dec 22, 2017
@scott967

This comment has been minimized.

Copy link

commented Jan 6, 2018

I think this has caused a problem in reading locale / regional settings. Changing temp unit or speed unit from default causes crash when loading giusettings on start. See Trac #17706

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Jan 6, 2018

@scott967 see #13319. Should be fixed in the next nightly.

@@ -1012,7 +1012,7 @@ void CSettings::InitializeISettingCallbacks()
settingSet.clear();
settingSet.insert(CSettings::SETTING_WEATHER_ADDON);
settingSet.insert(CSettings::SETTING_WEATHER_ADDONSETTINGS);
GetSettingsManager()->RegisterCallback(&g_weatherManager, settingSet);
GetSettingsManager()->RegisterCallback(&CServiceBroker::GetWeatherManager(), settingSet);

This comment has been minimized.

Copy link
@FernetMenta

FernetMenta Jan 7, 2018

Member

This is not correct. @Montellese this mechanism for registering callbacks is based on globals and has become an issue now. Here it registers a method that does not exist at this time.
We need a mechanism where services can register dynamically.

This comment has been minimized.

Copy link
@xhaggi

xhaggi Jan 7, 2018

Author Member

I didn't think of it while refactored the weather manager. We are able to call RegisterCallback from CWeatherManager. There is no need to leave it here.

This comment has been minimized.

Copy link
@FernetMenta

FernetMenta Jan 7, 2018

Member

Nice, I didn't know. Are you taking care of it?

This comment has been minimized.

Copy link
@xhaggi

xhaggi Jan 7, 2018

Author Member

see #13326

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.