Skip to content

Add language to dark sky weather component#15130

Merged
bachya merged 2 commits intodevfrom
weather-darksky-language
Jun 25, 2018
Merged

Add language to dark sky weather component#15130
bachya merged 2 commits intodevfrom
weather-darksky-language

Conversation

@pvizeli
Copy link
Copy Markdown
Member

@pvizeli pvizeli commented Jun 24, 2018

Description:

Add support for language in dark sky weather component.

@ghost ghost assigned pvizeli Jun 24, 2018
@ghost ghost added the in progress label Jun 24, 2018
Copy link
Copy Markdown
Contributor

@bachya bachya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall! Just a few small nits.

@@ -45,13 +60,14 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
latitude = config.get(CONF_LATITUDE, hass.config.latitude)
longitude = config.get(CONF_LONGITUDE, hass.config.longitude)
name = config.get(CONF_NAME)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you give this a default, you don't need .get: just access it directly.

latitude = config.get(CONF_LATITUDE, hass.config.latitude)
longitude = config.get(CONF_LONGITUDE, hass.config.longitude)
name = config.get(CONF_NAME)
lang = config.get(CONF_LANGUAGE)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you give this a default, you don't need .get: just access it directly.


dark_sky = DarkSkyData(
config.get(CONF_API_KEY), latitude, longitude, units)
config.get(CONF_API_KEY), latitude, longitude, units, lang)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is listed as required in the schema, you don't need .get: just access it directly.

@pvizeli
Copy link
Copy Markdown
Member Author

pvizeli commented Jun 25, 2018

In a future PR, I will fix also the icons for the weather UI.

@bachya bachya merged commit 672a3c7 into dev Jun 25, 2018
@ghost ghost removed the in progress label Jun 25, 2018
@pvizeli pvizeli deleted the weather-darksky-language branch June 25, 2018 15:54
@fabaff
Copy link
Copy Markdown
Member

fabaff commented Jun 25, 2018

How does this change work with our translations? I assume that the L10n support will break as the conditions no longer match. Only the sensors should support different languages not the weather platforms.

Looks like the same as #14608.

@c727
Copy link
Copy Markdown

c727 commented Jun 25, 2018

This only works for the current weather text. Forecast and icons are still "broken" as they are not mapped correctly

@fabaff
Copy link
Copy Markdown
Member

fabaff commented Jun 25, 2018

The proper fix is to do the mapping right.

@pvizeli pvizeli restored the weather-darksky-language branch June 25, 2018 17:03
@pvizeli pvizeli deleted the weather-darksky-language branch June 25, 2018 17:03
pvizeli added a commit that referenced this pull request Jun 25, 2018
pvizeli added a commit that referenced this pull request Jun 25, 2018
* Revert "Fix #14919. Should throw exception when camera stream closed by frontend (#15028)"

This reverts commit 508d045.

* Revert "Fix pylintrc section order and option placements (#15120)"

This reverts commit dbae410.

* Revert "Add storage helper and migrate config entries (#15045)"

This reverts commit ae51dc0.

* Revert "Add language to dark sky weather component (#15130)"

This reverts commit 672a3c7.
@balloob balloob mentioned this pull request Jul 6, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants