8000 Small amendments for Hash#merge with IndifferentAccess by yogeshjain999 · Pull Request #525 · hashie/hashie · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Small amendments for Hash#merge with IndifferentAccess #525

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

Merged
merged 1 commit into from
Jun 10, 2020

Conversation

yogeshjain999
Copy link
Contributor
  1. Use indifferent_writer in convert! so that when
    indifferent_writer, convert_key or indifferent_value is
    overridden in included class, merge can use those.

  2. convert! was calling twice if other hash was lacking
    indifference. IndifferentAccess.inject! already does conversion.

I've not written specs because both changes doesn't seem to be either fix or feature. But let me know if needed. 🥂

@dangerpr-bot
Copy link
dangerpr-bot commented Jun 6, 2020
1 Warning
⚠️ Unless you’re refactoring existing code or improving documentation, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#525](https://github.com/hashie/hashie/pull/525): Small amendments for hash#merge with indifferentaccess - [@yogeshjain999](https://github.com/yogeshjain999).

Generated by 🚫 Danger

@dblock
Copy link
Member
dblock commented Jun 8, 2020

How did you find this? I think the first one should be testable, since if you override any of these methods in the included class merge would miss them, no?

@yogeshjain999 yogeshjain999 force-pushed the indifferent-convert-change branch from b025025 to 747c9cb Compare June 10, 2020 03:33
@yogeshjain999
Copy link
Contributor Author

@dblock Right. How about the spec I've added ?
I found this when I was overriding writers in our gem for some custom needs.

1. Use `indifferent_writer` in `convert!` so that when
`indifferent_writer`, `convert_key` or `indifferent_value` is
overridden in included class, `merge` can use those.

2. `convert!` was calling twice if `other` hash was lacking
indifference. `IndifferentAccess.inject!` already does conversion.
@yogeshjain999 yogeshjain999 force-pushed the indifferent-convert-change branch from 747c9cb to b206fb8 Compare June 10, 2020 05:54
@dblock
Copy link
Member
dblock commented Jun 10, 2020

Love the use of grumpy and indifferent cat. Merging.

@dblock dblock merged commit c95b261 into hashie:master Jun 10, 2020
@dblock
Copy link
Member
dblock commented Jun 10, 2020

Now that I think about this I think there was a changelog line here, "Use indifferent_writer in convert!", it exposes a way to override it, but NBD.

@yogeshjain999
Copy link
Contributor Author

Ah right 😄

@yogeshjain999 yogeshjain999 deleted the indifferent-convert-change branch June 10, 2020 13:58
@dblock
Copy link
Member
dblock commented Jun 10, 2020

Ah right 😄

PR it. Let's at least ack your work in CHANGELOG!

@yogeshjain999
Copy link
Contributor Author

Thanks @dblock 🍻 Created PR #526 for it.

dblock added a commit that referenced this pull request Jun 11, 2020
Small amendments for Hash#merge with IndifferentAccess
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.

3 participants
0