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

UTF-8 support #128

Merged
merged 2 commits into from Mar 19, 2019
Merged

UTF-8 support #128

merged 2 commits into from Mar 19, 2019

Conversation

bonfante
Copy link
Contributor

@bonfante bonfante commented Mar 17, 2019

UTF-8 support.

Extension mbstring become required and php ^7.2 to use mb_ord

Require php ^7.2 in order to use mb_ord, fix style
src/Hashids.php Outdated Show resolved Hide resolved
src/Hashids.php Outdated Show resolved Hide resolved
src/Hashids.php Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
@vinkla
Copy link
Owner

vinkla commented Mar 18, 2019

Thanks for the pull request @bonfante! Please see my comments.

This pull request is featuring breaking changes and will require a new major release.

@ivanakimov any thoughts?

@vinkla vinkla self-assigned this Mar 18, 2019
@jd327
Copy link
Collaborator

jd327 commented Mar 18, 2019

@bonfante Thanks for the PR!
@vinkla Thanks for the quick follow-up -- I am following your lead on this one!

@bonfante
Copy link
Contributor Author

@vinkla done 👍.

@vinkla vinkla merged commit 2ad3643 into vinkla:master Mar 19, 2019
@vinkla
Copy link
Owner

vinkla commented Mar 19, 2019

Thanks @bonfante! ✨

We'll bump the branch alias to 4.0, please try using the new version in any project. We'll release a new version in a week or two.

@jfcherng
Copy link

jfcherng commented Apr 3, 2019

mb_ord() could be polyfilled thus no need to be PHP72-only actually.

https://github.com/symfony/polyfill-php72/blob/master/Php72.php#L191

@vinkla
Copy link
Owner

vinkla commented Apr 3, 2019

That is great! We’ll probably leave it as it is since 7.1 will reach EOL this year anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants