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

Second try #55

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Second try #55

wants to merge 3 commits into from

Conversation

ambs
Copy link
Contributor

@ambs ambs commented Aug 22, 2016

No description provided.

@ambs ambs mentioned this pull request Aug 22, 2016
@Leont
Copy link
Member

Leont commented Aug 22, 2016

The if utf8::is_utf8 has to go. Please forget that that function exists, pretty much all uses of it are wrong.

@@ -443,6 +443,11 @@ sub _CLASS {
) ? $_[0] : undef;
}

sub _to_bytes {
utf8::encode($_[0]) if utf8::is_utf8 $_[0];
Copy link
Member

@Leont Leont Aug 22, 2016

Choose a reason for hiding this comment

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

Why utf8::is_utf8?

@ambs
Copy link
Contributor Author

ambs commented Aug 23, 2016

Thanks, @Leont, for your changes.
I fixed now test 20 to use utf8 as well.
If all looks good, can we merge this one?

@mattias-p
Copy link

Is there anything happening for this PR? Any chance it gets merged?

For what it's worth I successfully tested 2f76d53. Unit tests pass and META.yml gets written in UTF-8 (I've got an author with U+00E4 characters in Makefile.PL).

@karenetheridge karenetheridge self-assigned this Jun 27, 2018
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.

4 participants