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

Remove implicit to_hash conversion from Config #114

Merged
merged 1 commit into from May 18, 2021
Merged

Conversation

timriley
Copy link
Member

This causes problems when passing config objects to methods with **splat-style "kwrest" keyword parameters, since it will result in the config object unintentionally being destructured into a hash.

I don't believe this to_hash behaviour is a critical aspect of what we're offering with dry-configurable. My feeling is that no one really expects a config object to be 100% substitutable with a hash, and getting a plain hash when required is still easy via #to_h or #values.

I've raised this PR because of failures in hanami/hanami#1107 for the tests running against Ruby 2.6: we pass one config object to another configurable object, and it ends up being deconstructed into a Hash, which is not expected and causes failures when we then try to reference settings by their named methods.

@timriley timriley requested a review from solnic as a code owner May 18, 2021 06:17
This causes problems when passing config objects to methods with `**splat`-style "kwrest" keyword parameters, since it will result in the config object unintentionally being destructured into a hash.
Copy link
Member

@flash-gordon flash-gordon left a comment

Choose a reason for hiding this comment

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

@timriley timriley merged commit 25ecf51 into master May 18, 2021
@timriley timriley deleted the remove-to-hash branch May 18, 2021 11:03
@timriley
Copy link
Member Author

Love a resounding approval! 😄

timriley added a commit to dry-rb/dry-schema that referenced this pull request Aug 29, 2021
The support for implicit conversion has been removed (dry-rb/dry-configurable#114) and will be included in the upcoming dry-configurable 0.13.0 release.
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

4 participants