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

Fix lux reporting and other attributes #4500

Closed
wants to merge 1 commit into from

Conversation

rboy1
Copy link
Contributor

@rboy1 rboy1 commented Jun 13, 2019

iconcode is provided as number, the switch case statement assumes it's a string and always defaults to 10000, fix switch case statements
percentPrecip and forecastIcon attributes are a string, convert numbers to string while reporting

@juano2310 @bflorian

`iconcode` is provided as number, the switch case statement assumes it's a string and always defaults to 10000, fix switch case statements
`percentPrecip` and `forecastIcon` attributes are a string, convert numbers to string while reporting
@tpmanley
Copy link
Contributor

tpmanley commented Jun 13, 2019

@bflorian actually had a similar change that hasn't been merged yet #4100. I'd prefer to go with that PR since it has a ticket. @rboy1 @juano2310 could you give that a review and/or test and then we can merge it?

@tpmanley
Copy link
Contributor

@rboy1 I merged that other PR into master with just the lux fix. Can you rebase your change on latest master for the attribute fixes?

@rboy1
Copy link
Contributor Author

rboy1 commented Jun 17, 2019

Done, made a new pull request, closing this one

@rboy1 rboy1 closed this Jun 17, 2019
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.

3 participants