Skip to content

Commit

Permalink
Don't specify behavior not supported by gson
Browse files Browse the repository at this point in the history
  • Loading branch information
@sferik
sferik committed Feb 10, 2013
1 parent 16ccaa1 commit db0d880
Showing 1 changed file with 0 additions and 9 deletions.
9 changes: 0 additions & 9 deletions spec/adapter_shared_example.rb
View file
Expand Up @@ -82,15 +82,6 @@ class << time
end
end

it 'dumps custom objects which implements to_json' do
klass = Class.new do
def to_json(*)
"\"foobar\""
end
end
expect(MultiJson.dump(klass.new)).to eq "\"foobar\""
end

it 'allow to dump JSON values' do
expect(MultiJson.dump(42)).to eq '42'
end
Expand Down

4 comments on commit db0d880

@rwz
Copy link
Member

@rwz rwz commented on db0d880 Feb 11, 2013

Choose a reason for hiding this comment

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

Shouldn't we skip this test only for gson? Or if we do not guarantee to_json to work across other adapters, we could remove some code from ok_json adapter, since it has a special case for to_json.

@sferik
Copy link
Member Author

@sferik sferik commented on db0d880 Feb 11, 2013

Choose a reason for hiding this comment

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

The whole point of MultiJSON is guaranteeing the same behavior across all adapters. This means we can't support behavior that isn't supported by all adapters. I'm okay with providing to_json as part of ok_json but we shouldn't specify that behavior if we can't guarantee it.

@rwz
Copy link
Member

@rwz rwz commented on db0d880 Feb 11, 2013

Choose a reason for hiding this comment

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

Ok, I'll prepare another pull-request removing redundant parts from ok_json adapter later today then.

@sferik
Copy link
Member Author

@sferik sferik commented on db0d880 Feb 11, 2013

Choose a reason for hiding this comment

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

You've convinced me to reverse my opinion on this. I'll add back this spec and wrap it with unless adapter == 'gson'.

Please sign in to comment.