Skip to content

Implement Hash#slice method#4949

Merged
enebo merged 1 commit intojruby:ruby-2.5from
ChrisBr:feature/array-slice
Jan 8, 2018
Merged

Implement Hash#slice method#4949
enebo merged 1 commit intojruby:ruby-2.5from
ChrisBr:feature/array-slice

Conversation

@ChrisBr
Copy link
Contributor

@ChrisBr ChrisBr commented Jan 7, 2018

for Ruby 2.5 support as in https://bugs.ruby-lang.org/issues/8499 stated.
#4876

Tests are already in master, https://github.com/jruby/jruby/blob/7bd10a61c70d7ab271f5a1403b64ef37eaaf036a/spec/ruby/core/hash/slice_spec.rb
Commit: 75e4522#diff-a265476d146c86ed0f4352a5735b7e4b

Do we want to cherry-pick the tests to ruby-2.5 branch?

Copy link
Member

@enebo enebo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny style requests but as you are contributing more I will slowly give you small changes like this to match general style.

*/
@JRubyMethod(name = "slice", rest = true)
public RubyHash slice(final ThreadContext context, final IRubyObject[] args) {
RubyHash result = newHash(getRuntime());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this to context.runtime?

for (int i = 0; i < args.length; i++) {
IRubyObject key = args[i];
IRubyObject value = this.internalGet(key);
if(value != null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (value != null) { // space after if and we only allow non-curly if it is all on one line.

@ChrisBr ChrisBr force-pushed the feature/array-slice branch from 5e74646 to cfae4b2 Compare January 8, 2018 17:04
@ChrisBr
Copy link
Contributor Author

ChrisBr commented Jan 8, 2018

@enebo thanks, updated!

@enebo enebo added this to the JRuby 9.3.0.0 milestone Jan 8, 2018
@enebo enebo merged commit fefd7fa into jruby:ruby-2.5 Jan 8, 2018
@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