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

Fixes #119: Allow overwriting methods in Mash #198

Conversation

michaelherold
Copy link
Member

Changes to Mash

When a method is overwritten, you can still access it by passing #hash_<method_name>. For an example, see the included documentation.

This is a breaking change since it changes the behavior of overridden methods, so it should require a version bump for Hashie to adhere to the Semantic Versioning policy.

Smash

Added as a safe alternative to Mash. A Smash prevents the user from overwriting a method with an attribute. This is more along the lines of the original Mash, except that it raises an ArgumentError if the user tries to overwrite a method with an attribute.

When a method is overwritten, you can still access it by passing
`#hash_<method_name>`. For an example, see the included documentation.

This is a breaking change since it changes the behavior of overridden
methods, so it should require a version bump for Hashie to adhere to the
[Semantic Versioning][semver] policy.

[semver]: http://semver.org
@gregory
Copy link
Contributor

gregory commented Jul 24, 2014

This is a quick reply, haven't dig into the code, but i don't really get the point of this PR.
Which issue/feature you are trying to fix/bring?

Could you explain why method access is not enough?

thx! :)

@dblock
Copy link
Member

dblock commented Aug 17, 2014

Bump @michaelherold

@michaelherold
Copy link
Member Author

I'll see if I can get to this this week. Got a lot going on at work.

Here's what I think needs done from the discussion in the related thread:

  • Update the README for Mash to not talk about wrapping JSON objects
  • Rework the functionality from this PR into an extension (name TBA)
  • Write a Mash extension that would raise a warning if trying to overwrite a method with a property

Does that sound about right?

/cc @gregory since he was a big part of the discussion around this PR
/cc @armchairdj since he was the reporter of the original issue

@armchairdj
Copy link

Sounds right to me.

michaelherold added a commit to michaelherold/hashie that referenced this pull request Aug 20, 2014
This is part 1 of 3 of the to-do list determined in hashie#198.
michaelherold added a commit to michaelherold/hashie that referenced this pull request Aug 20, 2014
This is part 1 of 3 of the to-do list determined in hashie#198.
michaelherold added a commit to michaelherold/hashie that referenced this pull request Aug 20, 2014
This is part 2 of 3 of the to-do list determined in hashie#198.
michaelherold added a commit to michaelherold/hashie that referenced this pull request Aug 20, 2014
This is part 2 of 3 of the to-do list determined in hashie#198.
michaelherold added a commit to michaelherold/hashie that referenced this pull request Aug 20, 2014
This is part 3 of 3 of the to-do list determined in hashie#198.
michaelherold added a commit to michaelherold/hashie that referenced this pull request Aug 20, 2014
This is part 1 of 3 of the to-do list determined in hashie#198.
@michaelherold
Copy link
Member Author

Okay, I've finished a first crack at all three of those to-do items in #203, #204, and #205. What do you think?

michaelherold added a commit to michaelherold/hashie that referenced this pull request Aug 20, 2014
This is part 2 of 3 of the to-do list determined in hashie#198.
dblock pushed a commit that referenced this pull request Aug 20, 2014
This is part 2 of 3 of the to-do list determined in #198.
dblock pushed a commit that referenced this pull request Aug 20, 2014
This is part 3 of 3 of the to-do list determined in #198.
@dblock
Copy link
Member

dblock commented Aug 20, 2014

Merged all those, great work! What do we want to do with this PR? Close?

@michaelherold
Copy link
Member Author

Great! 👍

All of the relevant stuff is split up between those other PRs, so I'm going to go ahead and close this one.

@michaelherold michaelherold deleted the allow-overwriting-methods-in-mash branch August 26, 2014 13:44
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