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

Added JSON serializer #37

Merged
merged 4 commits into from
Apr 28, 2023
Merged

Added JSON serializer #37

merged 4 commits into from
Apr 28, 2023

Conversation

TABeauchat
Copy link
Contributor

Just added the json serializer to allow from_json to be called.

Copy link
Contributor

@zeilhofe-xp360 zeilhofe-xp360 left a comment

Choose a reason for hiding this comment

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

Should we include tests like sobj.responds_to(:to_json)?

@TABeauchat TABeauchat requested a review from bfrey08 April 24, 2023 14:11
Copy link

@jacobdaddario jacobdaddario left a comment

Choose a reason for hiding this comment

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

This seems fine if there's a need for it. It's always nice to be able to re-use some existing Rails functionality. Obviously we don't want tests that are redundant with Rails, but perhaps we could just drop two lines into sObject to indicate that the class responds to as_json and from_json.

@asedge
Copy link
Collaborator

asedge commented Apr 28, 2023

@jacobdaddario @zeilhofe-xp360 Either of you want to push up those specs & add a CHANGELOG entry? I'm happy to approve/merge afterwards.

@zeilhofe-xp360
Copy link
Contributor

zeilhofe-xp360 commented Apr 28, 2023

@jacobdaddario @zeilhofe-xp360 Either of you want to push up those specs & add a CHANGELOG entry? I'm happy to approve/merge afterwards.

Added specs and CHANGELOG update

Copy link
Collaborator

@asedge asedge left a comment

Choose a reason for hiding this comment

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

LGTM!

@asedge asedge merged commit 1d315b0 into main Apr 28, 2023
@asedge asedge deleted the feature/from-json branch April 28, 2023 18:12
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.

4 participants