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

Implement Integer.sqrt #4904

Merged
merged 2 commits into from Dec 18, 2017

Conversation

Projects
None yet
2 participants
@nomadium
Contributor

nomadium commented Dec 18, 2017

Hi folks,

I was taking a look at open jruby issues and how to contribute with adding Ruby 2.5 support, so I prepared this small pull request.

I believe this is not a priority right now since Ruby 2.5 is not released yet, but I decided to contribute anyway, so I'm eager to receive feedback and check if I'm going in the right direction with this.

Please consider this as a WIP, since I could not see the results of the introduced tests in Travis although is working fine for me when I manually tested it in jirb.

Thanks,

nomadium added some commits Dec 17, 2017

Implement Integer.sqrt
For more information, please see feature #13219.

@enebo enebo added this to the JRuby 9.3.0.0 milestone Dec 18, 2017

@enebo enebo merged commit 60cc2ef into jruby:ruby-2.5 Dec 18, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
@enebo

This comment has been minimized.

Member

enebo commented Dec 18, 2017

@nomadium I believe the code is alright. The tests themselves we actually get upstream from MRI. So while I merged those tests in they will at some point get wiped out when we resync with MRI.

The solutions to this problem is either:
a) resubmit them to ruby-lang MRI project
b) rewrite them as specs for ruby/spec (which is getting a lot of contributions lately -- probably most preferred).
c) tell us you did enough hard work and we can try and push these tests upstream.

If these were gotten from upstream then I guess that is d) and we will get them next time we sync with their test suite :)

@nomadium

This comment has been minimized.

Contributor

nomadium commented Dec 18, 2017

@enebo Oh, good to know about the tests. Don't worry about it since I just copied them from MRI 2.5 test suite, so you are right about option d).

Thanks for reviewing and accepting the contribution so fast. :)

@enebo

This comment has been minimized.

Member

enebo commented Dec 18, 2017

@nomadium yeah thanks for the contribution. Keep going! :)

@nomadium nomadium deleted the nomadium:implement-integer-sqrt branch Dec 19, 2017

@nomadium

This comment has been minimized.

Contributor

nomadium commented Dec 27, 2017

Add link to the Ruby 2.5 support tracking issue: #4876

@enebo enebo modified the milestones: JRuby 9.3.0.0, JRuby 9.2.0.0 Apr 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment