Clean up multi_to_json #174
Conversation
|
This PR will be meaningless if #177 is merged |
|
@shuheiktgw I like #177 lets see what @bf4 says about the changes. They look good to me |
| _fast_to_json(object) | ||
| end | ||
|
|
||
| private |
There was a problem hiding this comment.
I think what you might mean here is to apply # @api private to the below methods, which I disagree with. It might make more sense to document intended usage, but I would say this is all a stable public API.
The private keyword has no effect on the interface.
In any case, the argument I can see for declaring it # @api private would be to say to anyone using the library not to rely on those methods being there, whether or not their public. (In Ruby, nothing's really private anyhow).
|
|
||
| def self.reset_to_json! | ||
| undef :_fast_to_json if method_defined?(:_fast_to_json) | ||
| logger.debug { "Undefining #{receiver}._fast_to_json" } |
There was a problem hiding this comment.
❓ It's not used in the normal lifecycle of the app and because there's no test in the suite, but I wouldn't say it serves no purpose. Especially if this module were made module_function and included.
I do have some tests I started writing in https://github.com/bf4/fast_multi_json but they didn't cover this specifically
|
@bf4 Thank you so much for your comments! I should probably reconsider about this PR... |
3 participants
I cleaned up
multi_to_json.rbin the following two ways.publicmethod private.self.reset_to_json!method