librato / librato-metrics

Ruby wrapper to make it easy to interact with Librato's API.
https://librato.com
Other
108 stars 51 forks source link

Fix SmartJSON #123

Closed chancefeick closed 7 years ago

chancefeick commented 7 years ago

Fix for https://github.com/librato/librato-metrics/issues/122. Add 2.3.1 to CI build matrix. Wrap methods instead of delegation. def_delegator raises a NoMethodError when adding delegation for objects:

RUBY_VERSION
# => "2.3.0"
require 'forwardable'
require 'json'

obj = Object.new
obj.extend SingleForwardable
obj.def_delegator JSON, :parse
obj.parse("{\"abc\":\"def\"}")
# => {"abc"=>"def"}
RUBY_VERSION
# => "2.3.1"
require 'forwardable'
require 'json'

obj = Object.new
obj.extend SingleForwardable
obj.def_delegator JSON, :parse
NoMethodError: undefined method `method_defined?' for #<Object:0x007f86888e7c48>

https://bugs.ruby-lang.org/issues/12393

nextmat commented 7 years ago

Yeah, I'm definitely not keen on wrapping a custom set of code for above a specific ruby ver. Let's chat about how to unify this in a way that works across all supported vers.

nextmat commented 7 years ago

LGTM :+1:

nextmat commented 7 years ago

Just thinking: we should probably release this as a patch-level release before #121 comes in for 2.1

chancefeick commented 7 years ago

@nextmat agreed, will ship this as 2.0.1