- Notifications
You must be signed in to change notification settings - Fork 524
Utf8 encode #1225
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
Utf8 encode #1225
Conversation
| Note Reference: https://www.php.net/manual/en/function.utf8-encode.php#refsect1-function.utf8-encode-notes Thanks a lot ; I indeed miss |
368807a to ef240ed Compare | Sure thing, that works as well (and is simpler). I wasn't sure what's the policy on BC changes. PR is updated as suggested. Thank you! |
| Thanks @kornrunner 👍 |
| Both failing checks (phpstan and php cs fixer) seem to be regarding code that hasn't been changed in this PR (GoogleMaps/BingMaps and GeoIP2). I can submit another PR regarding those - as I don't think they address current issue? EDIT: submitted #1226 |
Another option would be to require symfony/polyfill-mbstring to avoid breaks for apps that doesn't have mbstring installed |
| Changed requirement from |
9e0434a to e325817 Compare | As soon you've merged #1226 and rebased this PR, I'll approve it! Thanks a lot! 👍 |
| Sure thing, just - I have no privileges to merge it. Thanks! |
| Ah that's strange ... I've just done it 👌 |
| It's rebased, thank you! |
| Weirdly - it seems this change hasn't propagated to https://github.com/geocoder-php/ip2location-binary-provider ? |
| Indeed! I'll investigate. |
| IP2Location is indeed not in the list : https://github.com/geocoder-php/Geocoder/actions/runs/9142343555 There is probably a reason. I'll check and add it if it's an oversight. |
| 🔖 IP2LocationBinary version 1.3.1: https://packagist.org/packages/geocoder-php/ip2location-binary-provider#1.3.1 |
| Thank you! |
Trying to avoid
utf8_encodedeprecation warning on PHP 8.2+It will try to do the same using
mbstringoriconvfunctions depending on available extension, and eventually fallback to utf8_encode (not to break BC?).It's a bit of duplicated code - but I didn't think this specific case (used in two providers only) would warrant moving it to parent class.