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

(peazy) enumerator backports for 1.7 #4563

Merged
merged 8 commits into from Apr 20, 2017

Conversation

Projects
None yet
4 participants
@kares
Member

kares commented Apr 19, 2017

~ cherry-picked from master, so that feed and some others are available

resolves #1571

@kares kares added the JRuby 1.7.x label Apr 19, 2017

@kares kares added this to the JRuby 1.7.27 milestone Apr 19, 2017

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Apr 19, 2017

Member

@kares thanks for backporting/picking back to 1.7. My only concern with this work is adding 2.x semantics but I can see (I only checked seek) was added in 1.9.x. So long as compat is cool bombs away!

Member

enebo commented Apr 19, 2017

@kares thanks for backporting/picking back to 1.7. My only concern with this work is adding 2.x semantics but I can see (I only checked seek) was added in 1.9.x. So long as compat is cool bombs away!

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 19, 2017

Member

@enebo thanks, I did check the docs. feed, peek_values and next_values are there in 1.9.3

Member

kares commented Apr 19, 2017

@enebo thanks, I did check the docs. feed, peek_values and next_values are there in 1.9.3

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Apr 19, 2017

Member

@kares yeah I assumed you checked to see if the methods existed in 1.9. I was partially mentioning semantics of the methods themselves but I stated that perhaps in too general of a way.

Member

enebo commented Apr 19, 2017

@kares yeah I assumed you checked to see if the methods existed in 1.9. I was partially mentioning semantics of the methods themselves but I stated that perhaps in too general of a way.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 19, 2017

Member

👍 Looks good.

Member

headius commented Apr 19, 2017

👍 Looks good.

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 20, 2017

Member

ok, semantics isn't 100% MRI compatible like on 9K and since this is simply a back-port it will do the same.
Enumerator#feed acts different but it has been decided to be fine for 9K and it hasn't been changing in Ruby 2.x so that is why I thought its better than nothing esp. since there's an old bug report asking for it for 1.7

Member

kares commented Apr 20, 2017

ok, semantics isn't 100% MRI compatible like on 9K and since this is simply a back-port it will do the same.
Enumerator#feed acts different but it has been decided to be fine for 9K and it hasn't been changing in Ruby 2.x so that is why I thought its better than nothing esp. since there's an old bug report asking for it for 1.7

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Apr 20, 2017

Member

@kares yeah I was just asking for due dilligence on whether there was some 2.0ish behavior making back to 1.9. So as long as you thought about it I definitely trust your judgement.

Member

enebo commented Apr 20, 2017

@kares yeah I was just asking for due dilligence on whether there was some 2.0ish behavior making back to 1.9. So as long as you thought about it I definitely trust your judgement.

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 20, 2017

Member

@enebo great thanks. I am grateful as I am never 100% sure either on these. I'll squash and merge than.

Member

kares commented Apr 20, 2017

@enebo great thanks. I am grateful as I am never 100% sure either on these. I'll squash and merge than.

@kares kares merged commit c4198ee into jruby-1_7 Apr 20, 2017

0 of 4 checks passed

continuous-integration/appveyor/branch AppVeyor build failed
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment