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

Fix indexOf() RubyString method. #5715

Merged
merged 7 commits into from May 14, 2019
Merged

Conversation

@n00tmeg
Copy link
Contributor

@n00tmeg n00tmeg commented Apr 25, 2019

This PR is a possible fix for #5714 (Issue when splitting an encoded string with specific characters).
Please, refer to this issue for details.

resolves #5714

@kares
Copy link
Member

@kares kares commented Apr 25, 2019

look at CI, the change caused regressions: https://api.travis-ci.org/v3/job/524627557/log.txt (scroll at end)

Loading

@n00tmeg
Copy link
Contributor Author

@n00tmeg n00tmeg commented May 2, 2019

Thanks for the heads up, kares. I am closing this PR since my fix created another issue and needs to be investigated. I will resubmit a PR when it is done.

Loading

@n00tmeg n00tmeg closed this May 2, 2019
@n00tmeg n00tmeg reopened this May 6, 2019
@n00tmeg
Copy link
Contributor Author

@n00tmeg n00tmeg commented May 6, 2019

Hi @kares, I've fixed what was causing issue but tests are still failing. I am not able to reproduce these errors locally. Any idea of what could cause with these CI regressions?

Loading

@kares
Copy link
Member

@kares kares commented May 6, 2019

thanks - seems better. restarted some jobs.
one more thing, could you please move the tests so that you do not modify under test/mri
maybe just move them to a test/jruby/test_string.rb since they would get lost from next MRI update.

Loading

@n00tmeg
Copy link
Contributor Author

@n00tmeg n00tmeg commented May 6, 2019

Sure, I just moved the tests. Thanks!

Loading

@kares kares added this to the JRuby 9.2.8.0 milestone May 7, 2019
@headius
Copy link
Member

@headius headius commented May 14, 2019

Looks ok to me!

Loading

@headius headius merged commit 8748733 into jruby:master May 14, 2019
2 checks passed
Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants