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

JRuby compatibility #187

Merged
merged 18 commits into from
Mar 26, 2013
Merged

JRuby compatibility #187

merged 18 commits into from
Mar 26, 2013

Conversation

dwbutler
Copy link
Contributor

@dwbutler dwbutler commented Jan 3, 2013

Hi, I was thinking of using Paper Trail in a project that runs in JRuby. I noticed that JRuby isn't in the .travis.yml, so I modified some things so the project can bundle and run the tests in JRuby. It looks like the library itself works perfectly. I just had to make a modification to one test (the custom serializer) and some of the gems (JRuby has its own sqlite3 adapter you have to use).

Let me know what you think. Everything is green in Travis.

@bradleypriest
Copy link
Contributor

Does 42022be fix the need to change the custom serializer test?

@dwbutler
Copy link
Contributor Author

Unfortunately not. I merged in the latest changes and it would not pass in JRuby without my fixes applied.

The good news, though, is that switching over to use ActiveSupport::JSON instead of the JSON gem works consistently and "does the right thing." It's also more portable (for example, it will automagically pick up MultiJSON.)

By the way, using a JSON serializer is a common enough use case that I think it should probably be included in the gem. I can submit a separate pull request if you think that's a good idea.

@dwbutler
Copy link
Contributor Author

@batter
Copy link
Collaborator

batter commented Jan 22, 2013

@dwbutler - Thanks for the pull request. I'm going to try to merge it before the next release. Looks good to me, I just don't have any experience working with JRuby, so I wanted to tread a little more cautiously with this one.. perhaps try installing a local copy of it and playing around with it myself.

I also agree that having a JSON serializer packaged with the gem is a good idea, and was going to add one before the next release. I'll likely just re-use the one you've built for your tests here. Since this doesn't really tie directly into this request, I've filed a new issue for discussion, please see #194.

@ghost ghost assigned batter Jan 22, 2013
@dwbutler
Copy link
Contributor Author

Cool. I'm still learning the quirks of JRuby myself. If you have RVM set up, installing JRuby should (hopefully) be as simple as rvm install jruby.

@batter batter merged commit 995988d into paper-trail-gem:master Mar 26, 2013
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.

3 participants