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 failing spec on Ruby 3.0 #116

Merged
merged 1 commit into from
Aug 25, 2021

Conversation

andrehjr
Copy link
Member

Hi there 🙌

I think those specs are failing because Dir.glob is not being found over the C docs yard was generated from.

Investigating why I saw that Dir.glob was moved into dir.rb and removed from dir.c on the following commit ruby/ruby@60e25e3#diff-7b6c9d59e98b433bac23d954cfeb47fb25e36e93cdad448ffa777612fb52cc52

Changing this spec to a method still defined in C would make those specs pass and that could unblock the release?

Ideally for other $ Dir.glob and other methods moved for .rb files to work like it was on 2.7 there is a little bit more work. For example, adding those dir.rb and other files do the yardoc generation. I can investigate a bit more. Would that make sense?

@andrehjr andrehjr marked this pull request as ready for review August 21, 2021 18:54
@kyrylo kyrylo merged commit b8bf3b9 into pry:ruby-3-0-testing Aug 25, 2021
@kyrylo
Copy link
Member

kyrylo commented Aug 25, 2021

Nice one, that indeed unblocks the release.

@kyrylo
Copy link
Member

kyrylo commented Aug 25, 2021

That said, some of the tests fail on 3.0 :)
https://app.circleci.com/pipelines/github/pry/pry-doc/22/workflows/1f25c741-14b6-458a-991a-f1e0e71c9b5b/jobs/160

Here's the PR: #109

If that is merged, I will release a new version.

@andrehjr andrehjr deleted the update-specs-dir-c-methods branch August 25, 2021 12:10
@andrehjr
Copy link
Member Author

andrehjr commented Aug 25, 2021

@kyrylo that's odd, this commit is not merged on #109 anymore. (I made this PR on top of #109 / ruby-3-0-testing)

3.0 passed on this build https://app.circleci.com/pipelines/github/pry/pry-doc/21/workflows/67674a42-07b3-41c8-b678-c3603a805ffe 🤔 did you force push after?

Only Ruby 2.0 is failing on that build because of a rexml issue that also needs 👀 maybe because of ruby/rexml#69

@kyrylo
Copy link
Member

kyrylo commented Aug 25, 2021

That was the case - a force push. I didn't realize you wanted to merge that with branch ruby-3-0-testing. I thought it was merged into master.

@kyrylo
Copy link
Member

kyrylo commented Aug 25, 2021

I cannot cherry-pick your commit. Not sure how to restore it 😕
Do you mind opening a new PR, maybe?

@andrehjr
Copy link
Member Author

Done on #117 🙌

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.

2 participants