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

Pathname glob #5095

Closed
wants to merge 1 commit into from
Closed

Pathname glob #5095

wants to merge 1 commit into from

Conversation

headius
Copy link
Member

@headius headius commented Mar 19, 2018

Ported from MRI but does not appear to be passing the related test yet.

See #4876.

This does not yet pass MRI Pathname#glob test.
@headius headius added this to the JRuby 9.2.0.0 milestone Mar 19, 2018
@headius headius changed the base branch from master to ruby-2.5 March 19, 2018 20:19
@headius headius mentioned this pull request Mar 19, 2018
75 tasks
@nomadium
Copy link
Contributor

nomadium commented Mar 19, 2018

@headius I did something similar in #5085 but probably your change is more performant.

@kares
Copy link
Member

kares commented Mar 20, 2018

do like this one better - using the Stream APIs isn't necessary (believe they still have a hard time inlining)

@kares kares mentioned this pull request Mar 20, 2018
@kares kares added the ruby 2.5 label Mar 20, 2018
@headius
Copy link
Member Author

headius commented Mar 21, 2018

@nomadium @kares This is a direct port from MRI...but it doesn't pass the test 😢 If you get a chance maybe you can try to figure out why. I'm buried in other work right now.

@nomadium
Copy link
Contributor

nomadium commented Mar 22, 2018 via email

@nomadium nomadium mentioned this pull request Mar 22, 2018
@nomadium
Copy link
Contributor

@headius @kares I continued this PR on #5105 and got all the tests passing. You can review it when you have some time.

@kares
Copy link
Member

kares commented Mar 23, 2018

@nomadium najs, guess we can call this one a duplicate than. thanks!

@kares kares closed this Mar 23, 2018
@headius
Copy link
Member Author

headius commented Mar 23, 2018

Thank you!

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

Successfully merging this pull request may close these issues.

None yet

3 participants