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

Revert #182599 "add faster done for strings" #18275

Merged
merged 2 commits into from
Aug 29, 2016
Merged

Conversation

KristofferC
Copy link
Sponsor Member

My bad for merging without getting a proper review.

cc @tkelman

@tkelman tkelman added domain:unicode Related to unicode characters and encodings domain:strings "Strings!" labels Aug 29, 2016
@ViralBShah ViralBShah merged commit 6d179b3 into master Aug 29, 2016
@KristofferC KristofferC deleted the kc/rev_string_done branch August 29, 2016 12:58
@tkelman
Copy link
Contributor

tkelman commented Aug 29, 2016

probably shouldn't have squashed this, worth backporting the added test

TotalVerb added a commit to TotalVerb/julia that referenced this pull request Aug 29, 2016
@KristofferC
Copy link
Sponsor Member Author

As @TotalVerb pointed out, this test might not be good because it is creating an invalid UTF8 String. Perhaps the original fix is correct for all valid strings. But someone more knowledgeable than about UTF should probably comment.

@tkelman
Copy link
Contributor

tkelman commented Aug 29, 2016

okay, but while "what to do with invalid unicode" is still being worked out, I'm not going to backport any behavior changes right now

@KristofferC
Copy link
Sponsor Member Author

Yes, I just meant not backport the test.

@TotalVerb
Copy link
Contributor

See #18280.

mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
* Revert "add faster done for strings (JuliaLang#18259)"

This reverts commit 30ee10b.

* add test that failed with  JuliaLang#18259
stevengj pushed a commit that referenced this pull request Dec 8, 2016
* Revert "Revert #182599 "add faster done for strings" (#18275)"

This reverts commit 6d179b3.

* Test that next(s, endof(s)) > endof(s.data)
tkelman pushed a commit that referenced this pull request Mar 1, 2017
* Revert "Revert #182599 "add faster done for strings" (#18275)"

This reverts commit 6d179b3.

* Test that next(s, endof(s)) > endof(s.data)

(cherry picked from commit 62c3bfa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:strings "Strings!" domain:unicode Related to unicode characters and encodings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants