-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
Add Rails 4 compatibility under JRuby #102
Conversation
Thank you for this PR. To get Rails 4 working, I temporarily dropped JRuby support in master. At this point, your branch will be difficult to merge into master. Would you be willing to take a shot at adding back JRuby support to the master branch? I hate to say start over, but there are missing files, lots of whitespace changes, etc., so I suspect that a clean branch + some copypasta will be the best course. Rails 3.x is not supported in master, so the 3.x fix doesn't need to be included there. I will cherry-pick your Rails 3.x commit onto the 0.6-stable branch. |
I cherry-picked the 3.1 fix onto the 0.6-stable branch: a3f7e8604425404f |
Yup, I can definitely try to look at re-adding JRuby support to master. Getting everything working while supporting all the different versions of Rails was getting a little unwieldy, so I agree taking a fresh look will probably be easiest. If we're just targeting Rails 4 on master, I think JRuby support should be fairly straightforward to add. I'm not exactly sure when I'll be able to get to this, but hopefully sometime in the next week or two. Would you also be interested in a clean JRuby patch for the 0.7.x branch? It looks like that branch is for Rails 4.0 while master/1.0.0 is for Rails 4.1. Most of our JRuby projects that need postgis support are currently still using 4.0 and we haven't scheduled upgrades to 4.1 yet. So at least in the near term, I have in interest in getting JRuby support on both 0.7 and master. I can work on separate, clean PRs for each branch, but would you be interested in both? Or are you only interested in master at this point? But I'm excited to see things pick up again for this project. Thanks for becoming the new maintainer and a big thanks to @dazuma for everything he's done! |
Great. Yes, I would like to get patches for both Rails 4.0 & for Rails 4.1. If you want to work off the Thanks for your help! As an aside, I'd prefer not to have different branches for 4.0 & 4.1, but that's what happened. These 2 commits worked with 4.1 but not 4.0 & I just plowed ahead: |
@GUI This commit in master (add74ada5ffac) allows us to support 4.0 and 4.1 in master. So you can work off the master branch now. I'll be releasing this as 1.1.0 soon. |
How's the progress on this? I've just found out this project existed. I'm trying to work out if I should use this or raw SQL for queries for spacial things in JRuby on Rails 4.1. (I have no choice on platform). |
Hey guys, I'm also blocked on upgrading to rails 4 due to the same reasons as the previous comment! Whats involved in getting this merged? Do you need any help? Cheers. |
Sorry for the long delay in getting to this. This pull request should now be updated to bring JRuby compatibility with the current version of this gem. @teeparham Could you give this another look? Adding JRuby compatibility was actually way more straightforward now that this gem is only supporting Rails 4. There have also been some updates to the jdbc adapter in the meantime that also make this more straightforward. A few extra notes:
|
@GUI Thanks so much for doing this. I reviewed and it looks good to me. I agree: the overall diff of this PR is straightforward but the git history is not so readable. Here's a proposal that may be the easiest way to clean up the commit history:
You could copy over your versions of the changed files from your If that sounds reasonable, go for it & I'll merge asap. You could also squash/re-order commits via an interactive rebase, but I suspect it would be more work than simple copy/paste. Thanks again for you help! |
@teeparham Thanks for giving this a gander! I can definitely get this cleaned up into a more reasonable git commit history. I'll try to get that sorted later today or tomorrow at the latest. |
@teeparham Okay, I think this is all squashed and split up now into 3 separate PRs with single commits:
#102 and #103 aren't passing Travis, due to the aforementioned Rubinius issue, which I made a distinct PR. But once they all come together, I think Travis should be happy. I tried to structure PRs to minimize any merge conflicts if you want to take them all, but one thing to note is that after merging the JRuby and Appraisal stuff together, you'll need to generate new gemfiles (since the appraisal branch's gemfiles don't have the jruby gems included). Appraisal should make this easy, so once both are merged in, you can do this with Thanks! |
Thanks @GUI! Rails 4 JRuby folks: please try the latest master. I'll release version 2.2.0 soon. 🎆 |
Awesome, thanks so much for the quick merge! I'll try to get a couple of our JRuby apps updated to this final merged version that's on master this week to give it some additional testing. |
This adds Rails 4 compatibility under JRuby and the activerecord-jdbc-adapter. This addresses #76. Fixing this mostly involved making the adapter skip the OID usage stuff, since the jdbc adapter doesn't utilize that.
This also fixes an issue I discovered while running the test suite: Rails 3.1 compatibility was broken under JRuby if you were using the activerecord-jdbc-adapter 1.3.x (but it still worked fine under 1.2.x). I'm not sure how many people are actually using this likely obscure combination of versions, but the fix was relatively straightforward, so I fixed that too.
Note that the Rails 4 tests will currently fail on Travis due to this rgeo-activerecord issue: rgeo/rgeo-activerecord#12 But that issues affects all ruby platforms under Rails 4, not just JRuby. So I don't think you'll get clean tests until that issue is addressed in rgeo-activerecord, but in my own local testing, I get clean tests when building against that rgeo-activerecord branch.