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

Restore MultiJson and add custom option modules. Fix #303 #304

Merged
merged 7 commits into from
Sep 30, 2015

Conversation

jondeandres
Copy link
Contributor

In order to support JRuby it'd be better not force installing Oj since
it cannot be used in JRuby if C extensions are not enabled, and they are
not supported by default.

So, we add MultiJson again to the project. This will create more
different scenarios but since it could be the best option according to
previous paragraph.

We've added in this change support for custom options depending on the
loaded adapter in MultiJson. We can add more adapter options modules in
this way:

module Rollbar
  module JSON
    module AnyMultiJsonAdapterName
      def self.options
        { # here comes the options we need for that adapter }
      end
    end
  end
end

We have, for now, just the Oj module that will return the options we had
before this commit.

In order to support JRuby it'd be better not force installing Oj since
it cannot be used in JRuby if C extensions are not enabled, and they are
not supported by default.

So, we add MultiJson again to the project. This will create more
different scenarios but since it could be the best option according to
previous paragraph.

We've added in this change support for custom options depending on the
loaded adapter in MultiJson. We can add more adapter options modules in
this way:

module Rollbar
  module JSON
    module AnyMultiJsonAdapterName
      def self.options
        { # here comes the options we need for that adapter }
      end
    end
  end
end

We have, for now, just the Oj module that will return the options we had
before this commit.

The specs run with Oj by default and it's the recommended adapter to use
with Rollbar.
@runlevel5
Copy link
Contributor

Not a big fan of this PR but pragmatically it is 👍 to do

@jondeandres jondeandres force-pushed the restore-multi_json-and-custom-options branch 3 times, most recently from dd30c02 to 9b9c14d Compare September 21, 2015 22:54
- Add a Ruby extension that will install Oj when the platform is not
  JRuby
- Rollbar::JSON will force to use Oj if it's defined. If not it'll use
  the MultiJson.current_adapter

In summary. We'll use Oj for platforms != JRuby and any other one for JRuby.
@jondeandres jondeandres force-pushed the restore-multi_json-and-custom-options branch from 9b9c14d to e33f10e Compare September 21, 2015 22:58
jondeandres added a commit that referenced this pull request Sep 30, 2015
…tions

Restore MultiJson and add custom option modules. Fix #303
@jondeandres jondeandres merged commit 0a0da3b into master Sep 30, 2015
@jondeandres jondeandres deleted the restore-multi_json-and-custom-options branch September 30, 2015 14:45
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