drop okjson in favor of multi_json

[geemus]

No description provided.

[jkakar]

This looks good to me, +1! Do we normally update changelog.txt or
is that something we only do when we cut a release?

[geemus]

Historically I've just waited and updated changelog at release time. Open to discussing if that should change.

[jkakar]

No need to change, I just wanted to understand the process, thanks for
explaining.

[geemus]

Sure thing. That is more or less the process I use on everything that I manage releases for and it seems to work well-enough.

[wuputah]

👎. MultiJson should not be a dependency. There' s a number of easy alternatives, such as:
https://github.com/heroku/heroku-bouncer/blob/master/lib/heroku/bouncer/json_parser.rb

(This particular code above does not deal with error handling, but it wouldn't be hard to add that.)

If nothing else, the version constraint needs to be relaxed (~>1.8.2 is much too strict). I'd propose ~>1.5.

[geemus]

What is wrong with multijson? Also, I guess I'm concerned that the difficulty of getting other jsons installed inside the toolbelt environment might be bad. Anyway, would be helpful to have a deeper understanding of where you are coming from here. Thanks!

[wuputah]

For any gem, adding a dependency is a big deal - you're asking everyone everywhere to add that gem. The heroku-api gem is still the easiest way to use the Heroku API in your Ruby app, so it isn't just for the toolbelt. For instance, I use it in Help app. So it should only be if necessary. And MultiJson is, IMO, never necessary.

MultiJson (and Ruby+Json in general) is actually a huge problem, and is actually completely deprecated now -- it's use in the Ruby community is no longer recommended, and will be removed in Rails 4.1. I can't make an official recommendation on what to do at this time for the toolbelt, but I'd recommend talking to Terence. Read this, for a start:
intridea/multi_json#138 (comment)

For me, I've gone to some length to make it so that MultiJson doesn't actually get loaded into a project. You'll note that I use the lowest possible version number here (1.5.x) for compatibility with other gems that have a MultiJson dependency (I've since shipped some bug fixes, so its 1.5.3). Maybe that's dumb, but basically I initially used 0.0.1, and then had to bump it to 1.5.0 to get it to satisfy a dependency. In any case, unless heroku-api needs 1.8.x for some reason, you should really relax the dependency.

[wuputah]

(Edits and such are above, should you be reading your email instead of github.com. I always edit a lot. Including this. Sorry.)

[geemus]

I was reading the email. UG.

@hone - advice?

[hone]

@geemus what's the motivation here? performance? why use many different adaptors?

[geemus]

@hone - I believe there are multiple issues. There are some encoding issues related to non-ascii values that okjson chokes on, so we'd like to improve that situation (especially in toolbelt). On the heroku.rb side, in addition to that, we want to not get in the way/conflict with folks if they want to have other json stuff in their app that includes heroku.rb (and allow for better performance to be opted-in to). So partly performance, partly encoding and partly flexibility despite conflicting monkey patches.