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

Replace Hashie::Mash with ActiveSupport::HashWithIndifferentAccess. #1610

Merged
merged 4 commits into from
Apr 14, 2017

Conversation

dblock
Copy link
Member

@dblock dblock commented Apr 9, 2017

Replaces #1594, includes a way to set the class name globally.

@dblock
Copy link
Member Author

dblock commented Apr 9, 2017

@namusyaka @marshall-lee @james2m Would appreciate a careful code review and comments/feedback, I'll iterate quickly.

I am a little concerned that we're losing method access, since it probably will affect quite a few people but I think it's the right thing to do.

@dblock
Copy link
Member Author

dblock commented Apr 9, 2017

💚

@james2m
Copy link
Contributor

james2m commented Apr 9, 2017

@dblock thanks for finishing that up, looks similar to what I have locally, I just didn't have time to polish up and complete as I'm traveling until 17th. The cleaned up tests is a big relief too! 😬

@dblock
Copy link
Member Author

dblock commented Apr 10, 2017

You've done an excellent job up this far @james2m, thanks for your big effort!

@namusyaka
Copy link
Contributor

💚 👍
I also agree that method access of parameters isn't good design.

@namusyaka namusyaka added this to the 1.0.0 milestone Apr 11, 2017
@dblock
Copy link
Member Author

dblock commented Apr 14, 2017

How are we feeling about merging this?

@james2m
Copy link
Contributor

james2m commented Apr 14, 2017

I'm happy with it. Using it in a new app with Rails 5.1 release candidate. If I had more tinkering time I'd probably reorganise the files under /extensions as it feels like a dumping ground right now, but nothing functional.

@dblock dblock force-pushed the hash-with-indifferent-access branch from e27a374 to 717faed Compare April 14, 2017 17:19
@dblock dblock force-pushed the hash-with-indifferent-access branch from 717faed to d931da3 Compare April 14, 2017 17:21
@dblock dblock merged commit 91c6c78 into ruby-grape:master Apr 14, 2017
@dblock
Copy link
Member Author

dblock commented Apr 14, 2017

I merged this, lets see what happens ;)

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