Skip to content

Updated the UVIndex implementation #132

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

Merged
merged 13 commits into from
May 29, 2019
Merged

Updated the UVIndex implementation #132

merged 13 commits into from
May 29, 2019

Conversation

roelvanhintum
Copy link
Contributor

No description provided.

@cmfcmf
Copy link
Owner

cmfcmf commented May 6, 2019

Hi @roelvanhintum!
Thank you for your PR – would you be able to also take a look at the failing tests?

https://travis-ci.org/cmfcmf/OpenWeatherMap-PHP-Api/jobs/522113766#L499

@roelvanhintum
Copy link
Contributor Author

@cmfcmf i've changed the tests, but travis-ci is still having some problems with composer i can't figure out.

@cmfcmf
Copy link
Owner

cmfcmf commented May 15, 2019

Thank you for adjusting the tests. The build was failing on hhvm, which Composer doesn't support any longer. I therefore removed support for hhvm.

I'm sorry this takes so long - I'm hesitant to merge this as this, because this would break backwards compatiblity with the current implementation, if I'm reading the changes correctly. I hope to find time next week to take another look at your PR and possibly make it backwards compatible (maybe via an opt-in switch).

@roelvanhintum
Copy link
Contributor Author

I couldn't match the old implementation with the API, which was the reason i changed it.
https://openweathermap.org/api/uvi

Maybe this should be pushed to a major release, to make clear things can break?

@cmfcmf
Copy link
Owner

cmfcmf commented May 29, 2019

I've gone through this PR and updated some minor bits and pieces. I also fixed historic uv index retrieval and tests. This should be good to go now.
I will release this, as you suggested, as part of a new major version: 3.0.0. Before that, I'll fix and improve some other bits and pieces in #134 based on #72.

Thank you again for your work and patience, @roelvanhintum!

@roelvanhintum
Copy link
Contributor Author

@cmfcmf You're welcome and thank you!

@cmfcmf cmfcmf merged commit fdac750 into cmfcmf:master May 29, 2019
@cmfcmf cmfcmf mentioned this pull request May 29, 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.

2 participants