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

[Ruby 3.0 support] Add sorting for Dir.glob #2523

Merged
merged 4 commits into from
Nov 9, 2021

Conversation

Strech
Copy link

@Strech Strech commented Nov 3, 2021

I've started the check and it turns out that sorting is not yet implemented in Dir.glob in both default and extended behaviour (sort: Boolean version).

I start adding flags and also add new test case for non-false value of the sort option ruby/spec#894

@Strech Strech changed the title [Ruby 3.0 support] Add initial interface for Dir.glob sorting [Ruby 3.0 support] Add sorting for Dir.glob Nov 3, 2021
@eregon
Copy link
Member

eregon commented Nov 3, 2021

Ah, @aardvark179 removed it in 56ed1c2, I forgot.
Basically you need to add it back there, and pass the sort: value until there (can be via an extra argument or flags).

@Strech
Copy link
Author

Strech commented Nov 3, 2021

Thanks @eregon will check also some other specs regarding respecting {} order

@Strech Strech force-pushed the add-dir-glob-sorting branch from 66a2eaf to 33f7056 Compare November 8, 2021 21:04
@Strech Strech marked this pull request as ready for review November 8, 2021 21:05
@Strech
Copy link
Author

Strech commented Nov 8, 2021

@eregon I decide to go with flags for internal ruby code to keep just one interface there.

Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@eregon eregon added the in-ci The PR is being tested in CI. Do not push new commits. label Nov 9, 2021
@eregon eregon added this to the 22.0.0 milestone Nov 9, 2021
@eregon eregon force-pushed the add-dir-glob-sorting branch from 20a2c93 to ffa39a7 Compare November 9, 2021 17:10
@graalvmbot graalvmbot merged commit a4298d7 into oracle:master Nov 9, 2021
@Strech Strech deleted the add-dir-glob-sorting branch November 9, 2021 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-ci The PR is being tested in CI. Do not push new commits. oca-signed shopify
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants