-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Feature#12217 Enumerable/Array sum #4297
Conversation
@phluid61 I do have two questions. I suspect there are MRI tests we will now pass but I did not try it out so I don't know. Is that true? Secondly, can you check to make sure the sub is really an optional arity for Enumerable.sum? |
There are tests, yes:
I am in the process of porting them across, so I can run them and make sure it all passes. Regarding the arity, I think so. That means that this is allowed, right?
Or have I misunderstood what the arity means in that context in JRuby? |
@phluid61 yeah that probably means it accepts any number of arguments. Zero would not work if it was required to be 1. sum { |a,b,c| 1 } should also work and I am sure it does based on that. BTW we should also have those imported at test/mri/ruby/test_*.rb. We also can have excludes omitting tests in test/mri/excludes. You will see a name matching the class in those files with the method names which we will exclude (just letting you know as I do not see any excludes for test_sum). |
I've got my jruby branch built and executing. There are problems:
|
Well, that was ugly. I got MRI's enum tests to pass, and then I piggy-backed Array's implementation on it. The spec/test coverage isn't astounding, but I'm pretty sure I understood the MRI implementation well enough to catch the edge-cases. |
I completely rebuilt Array#sum to better match MRI's implementation. Then I created my own fake
|
Adds
Enumerable#sum
andArray#sum
from Feature #12217.I'm parking this PR here in its current state to encourage discussion and feedback.