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 count_spec (1.7.x) #1231

Closed
wants to merge 1 commit into from
Closed

Conversation

dmarcotte
Copy link
Contributor

Use the appropriate arity to get Enumerable#count to handle its args correctly. Note that we need to switch from JavaInternalBlockBody to a BlockCallback since JavaInternalBlockBody does not currently get its arguments prepared correctly for the given arity (this is fixed in master).

This does not need to be merged into master since #1230 takes care of it.

Use the appropriate arity to get Enumerable#count to handle its args
correctly.  Note that we need to switch from JavaInternalBlockBody to
a BlockCallback since JavaInternalBlockBody does not currently get its
arguments prepared correctly for the given arity.
@headius
Copy link
Member

headius commented Nov 17, 2013

There seems to be a conflict on the 1.7 branch with another commit of yours. Can you look into that? It appears that the count18 path is using 1.9 behavior, calling #size if present.

@dmarcotte
Copy link
Contributor Author

The call to #size is actually intended to be 1.8-specific (more details in #979). Was your concerned that it had ended up in the wrong place? Or am I missing something here? (Apologies if I am!)

@headius
Copy link
Member

headius commented Nov 17, 2013

If that's appropriate behavior for 1.8, then there's no problem. See if you can tidy up the PR and we'll pull it in.

@dmarcotte
Copy link
Contributor Author

Okay, now I'm definitely missing something :)

I'm not sure what to tidy... the pull looks correct to me. Can you hit me with a bit more detail on what you're seeing? I'd be glad to fix anything up.

@enebo
Copy link
Member

enebo commented Nov 19, 2013

cherry-picked this since this ui implies this was against master and not jruby-1_7.

@enebo enebo closed this Nov 19, 2013
@dmarcotte
Copy link
Contributor Author

Ah! That explains the confusion... my bad for sending this to the wrong branch by accident. Thanks for clearing things up @enebo.

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.

None yet

3 participants