rgeo / activerecord-postgis-adapter

ActiveRecord connection adapter for PostGIS, based on postgresql and rgeo
Other
863 stars 230 forks source link

Inquiry on Rails 7.2 Support Effort #395

Open seuros opened 3 months ago

seuros commented 3 months ago

Hi everyone,

I wanted to check if anyone is currently working on adding support for Rails 7.2. If there is already an effort underway, I would love to know more about it and see if there's any way I can assist.

If no one is handling this task, I am planning to dedicate some time over the weekend to work on it.

Remember this will be again a breaking change since the new version will support just 7.2.x

theonlyriddle commented 3 months ago

🥳 This would be very helpful!

I just updated to Rails 7.2 and found that my application wouldn't boot up. We've tentatively narrowed down the culprit to this gem.

anthony0030 commented 3 months ago

@seuros that would be sooo Cool, I was debugging an app all day yesterday and came to the conclusion that just changing the version number in this gem was not enough to get it working.

ugisozols commented 3 months ago

👋 I was also looking into this and got my Rails app booting with the changes in https://github.com/rgeo/activerecord-postgis-adapter/pull/396. I do not know if that's enough at this point as my Rails app needs many more updates to run on 7.2, so take it as is.

keithdoggett commented 3 months ago

Hey everyone thanks for the help in getting this up! Unfortunately I will not have much time to contribute for the next few weeks but will have time to review/make a release once we have a working version.

@ugisozols thanks for starting the PR. I haven't reviewed it yet, but we can work off of that branch for now and if necessary I can make a different dev branch people can fork off of.

BuonOmo commented 2 months ago

@seuros it looks like the fix of require rgeo/activerecord was not enough (https://github.com/rgeo/activerecord-postgis-adapter/actions/runs/9805704288/job/27075934099). Do you think you would have time to fix that ? I think We'll have to wait for this before we can ship unfortunately :/

seuros commented 2 months ago

@BuonOmo That a flaky test happening only in CI and don't affect runtime (the connection take some time to get established), restart the specific build it will pass.

As i said in the other discussion, i will fix it in a separate PR.

StoneGod commented 2 weeks ago

Hi! What about releasing new version?

StoneGod commented 2 weeks ago

@seuros Hi! what about release new version of gem? https://github.com/rgeo/activerecord-postgis-adapter/pull/403 rails 7.2 already released

totus commented 2 weeks ago

Same Q here

seuros commented 2 weeks ago

The code is already merged in this repo, if you could use github path to run you whole test suite that will be helpful.

We don't want to release something that is not battle tested.

The test suite is flaky, but works locally.

JamesChevalier commented 2 weeks ago

The code is already merged in this repo, if you could use github path to run you whole test suite that will be helpful.

We don't want to release something that is not battle tested.

The test suite is flaky, but works locally.

I bumped my Rails version in my Gemfile:

gem "rails", "~> 7.2.1"

And pointed activerecord-postgis-adapter to GitHub:

gem "activerecord-postgis-adapter", github: "rgeo/activerecord-postgis-adapter"

And my test suite produces the error reported in https://github.com/rgeo/activerecord-postgis-adapter/issues/402 :

TypeError:
  can't cast RGeo::Geographic::SphericalLineStringImpl

I have some details that might be helpful that I'll share in that Issue.

mjy commented 2 weeks ago

For what it's worth we have a 7.2 unofficial PR branch in CI for our app, see https://github.com/SpeciesFileGroup/taxonworks/pull/4006.

There are quite a few geo specs in the suite (though I wouldn't recommend them as a model for how to do things, we have another branch refactoring the whole approach), and the failures we do have seem unrelated.