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 test coerce2 #5006

Merged
merged 4 commits into from
Jan 24, 2018
Merged

Fix test coerce2 #5006

merged 4 commits into from
Jan 24, 2018

Conversation

yui-knk
Copy link
Contributor

@yui-knk yui-knk commented Jan 24, 2018

No description provided.

Currently `Numeric#%` is defined in the same way
as `Numeric#modulo`.

Ref: https://github.com/ruby/ruby/blob/v2_3_0/numeric.c#L4179-L4180
Currently these methods are not defined on `Rational`.
Currently this method is not defined on `Rational`.
Now `test_coerce2` does not fail.
@enebo enebo added this to the JRuby 9.1.16.0 milestone Jan 24, 2018
@enebo enebo added the core label Jan 24, 2018
@enebo
Copy link
Member

enebo commented Jan 24, 2018

@yui-knk just waiting for test:mri to finish and this looks good. Weirded out that '.div' is no longer there but I see it is not in Ruby docs for 2.2? I don't know the history of that merthod though.

@yui-knk
Copy link
Contributor Author

yui-knk commented Jan 24, 2018

Maybe ruby/ruby@d82ed7e this commit removed Rational#div and this commit was included Ruby 1.9.2, so I think Ruby docs for 2.2 should not contain that method.
I can not read why this method was removed by the commit message....

@enebo
Copy link
Member

enebo commented Jan 24, 2018

@yui-knk yeah it is strange to see Ruby API shrink :)

@enebo enebo merged commit 6b375fd into jruby:jruby-9.1 Jan 24, 2018
@yui-knk yui-knk deleted the fix_test_coerce2 branch January 24, 2018 23:52
kares added a commit that referenced this pull request Jan 31, 2018
Fix test coerce2 for master branch

since merge of #5006 wasn't handled properly due changes in RubyRational.java
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.

2 participants