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

Add an optional create additions callback #182

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

movitto
Copy link

@movitto movitto commented Sep 13, 2013

Follow up to workarounds describe in issue #179.

If not set create_additions still implements current behaviour.

If set to a proc this will be invoked in lieu of deep const get to evaluate the json class string and return a ruby class. It is up to the developer to specify the implementation details of this mapping.

Putting this out there for comments and hopeful merger into the codebase.

If specified json will pass the string json_class
and use the value returned as the ruby class to
instantiate
@movitto
Copy link
Author

movitto commented Sep 21, 2013

Hey just updated the patches to work on travis. Wasn't anything major, just had the wrong types for create_additions in the extensions. The builds that are failing are the allowed failures for rubinius as well as the latest ruby head (which looks to be a ruby issue as it's not even getting to installing the deps nevermind testing json itself). I'm assuming travis integration is acceptable at this point.

If there is anything else that needs TBD just shout out, would love to see this make it in so I don't have to implement one of the workarounds in rjr.

Thanks.

Copy link
Contributor

@marcandre marcandre left a comment

Choose a reason for hiding this comment

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

@movitto Great PR, I'm sorry that noone reviewed this before today.

This seems like a worthy addition.

Test needs to be a bit more detailed, and it is missing documentation.

@hsbt is the feature acceptable to you?

a = A.new(666)
assert A.json_creatable?
json = generate(a)
a_again = JSON.parse(json, :create_additions => proc { |e| A2 })
Copy link
Contributor

Choose a reason for hiding this comment

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

A test is missing on what is e

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

2 participants