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

Implement Integer.sqrt #4904

Merged
merged 2 commits into from
Dec 18, 2017
Merged

Conversation

nomadium
Copy link
Contributor

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,

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
@enebo
Copy link
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
Copy link
Contributor Author

@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
Copy link
Member

enebo commented Dec 18, 2017

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

@nomadium nomadium deleted the implement-integer-sqrt branch December 19, 2017 18:54
@nomadium
Copy link
Contributor Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants