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.

Sign up for GitHub

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

Jump to bottom

Jruby support #199

Merged
merged 5 commits into from
Nov 19, 2015
Merged

Jruby support #199

merged 5 commits into from
Nov 19, 2015

Conversation

deivid-rodriguez
Copy link
Member

Hi! This is an attempt to get the tests passing for JRuby. The main issue is 3d00764, since JRuby and MRI behave differently here and I don't know what's the expected behavior and whether this should be considered public API at all.

Opinions?

EDIT: Just linking to the original issue: #198

David Rodríguez added 4 commits November 19, 2015 20:28
Don't use driver internals for tests
f4d2964 
The `result_status` method is not provided by the Java driver. This
assertion was added in e4e3442, and the
intention was only to simplify the test, so I think we're fine reverting
that specific change.
Don't test activerecord internals
3d00764 
While in MRI default values kept in the `columns` array are always
quoted, in JRuby, they are resolved to the correct type. Since the
following test already checks for user expectation (correct default when
creating an object), I just removed the failing test.
Remove jruby from allowed failures
52db45c
I don't think we need "19 mode"
c9fb1e2 
JRuby 1.7.x is "1.9" compatible by default. Let's just use "jruby-1.7" and let
`rvm` match that against the latest available patch level version.
@deivid-rodriguez deivid-rodriguez force-pushed the jruby_support branch 2 times, most recently from e252819 to 925625d Compare November 19, 2015 20:53
@teeparham
Copy link
Member

Thank you! 🎉 👏

@teeparham teeparham mentioned this pull request Nov 19, 2015
Closed
@deivid-rodriguez
Copy link
Member Author

Fuzzy matching does not work... jruby-9.0 is matched against a prerelease version, while just jruby matches it against jruby-1.7. We need to be explicit.

teeparham added a commit that referenced this pull request Nov 19, 2015
@teeparham
Merge pull request #199 from deivid-rodriguez/jruby_support
7779232 
Jruby support
@teeparham teeparham merged commit 7779232 into rgeo:master Nov 19, 2015
@deivid-rodriguez deivid-rodriguez deleted the jruby_support branch November 19, 2015 21:08
@deivid-rodriguez
Copy link
Member Author

Thanks! 😄

I'd like to ask @kares though about this difference in behavior because whether this klass.columns trick is supported or not, I've seen it used in some places, so it'd be good to know what to expect.

@kares
Copy link

kares commented Nov 19, 2015

hey! honestly do not know if its much of an issue - recall noticing some column.default incompatibilities with MRI on 4.2 but I did not care much (we get type-casted results from elsewhere as well) esp. as the same column code supports older AR versions and no one objected so far. so it is as it is until it is 🙉

@deivid-rodriguez
Copy link
Member Author

Thanks! Sounds reasonable!

@teeparham
Copy link
Member

This PR is included in the 3.1.0 gem release: https://rubygems.org/gems/activerecord-postgis-adapter

JRuby folks: please give it a try & report any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@deivid-rodriguez @teeparham @kares