search

postgis / postgis-java

java bindings for postgis
149 stars 56 forks source link

Update JTS to 1.16.0 #70

Closed nitehawk42 closed 4 years ago

nitehawk42 commented 5 years ago

I upgraded JTS to 1.16.0 and things seem to be working ok.

There were some superficial changes to the constructors of PackedCoordinateSequence.Double()

https://github.com/nitehawk42/postgis-java/commit/89cc0876279ab5ede6a892bb006ddc9f82932b4d

TimPigden commented 4 years ago

Hi - I can see there's a PR for this - but it's over a year old. Is anyone doing anything about it? This sub-project is looking suspiciously moribund, which seems weird given how important a part of the whole java/gis infrastructure this is.

phillipross commented 4 years ago

You're right, there is a PR for this that's very old. I distinctly remember commenting that the PR should probably be updated to use the 1.16.1 artifact, but looking at the PR now, I don't see the comment 🤔

I'll comment again and ping @nitehawk42 to see if he'll update PR

nitehawk42 commented 4 years ago

I can update my PR, but I have been treating this project as abandoned and just use my own fork. I haven't hit any other bugs in the last year, though.

nitehawk42 commented 4 years ago

I also am not sure if the unit tests even cover the change I made, so I can't make any guarantees on it working. Luckily it is pretty easy to just fork my fork and run maven on it yourself.

nitehawk42 commented 4 years ago

I ran a quick test and simply replacing JTS 1.6.0 to 1.6.1 does work, but I can't provide a binary for it, so you will need to run maven to build Postgis, then install it into your local maven repository using install:install-file or your own server such as Nexus.

https://maven.apache.org/plugins/maven-install-plugin/install-file-mojo.html

One more note: My project is using Java 11, so I only ran this against Java 11. (For me, this mess was set off when I upgraded my project to Java 11 and Spring/Hibernate 5.x.)

phillipross commented 4 years ago

The project isn't abandoned, but it definitely hasn't seen much life in the way of contributions or new features. It would be nice to put some effort into getting it hooked up for automated testing via something like TravisCI, but we've been waiting for suggestions from the community (if there is any).

I'm not sure what binary you're referring to. If you still have the branch open, it should be as as simple as updating the pom.xml to reference the latest version of the artifact, and if it builds without breaking the existing tests that are run as part of the build then I would say it's fine. I just wanted to give you the opportunity to do it and treat it as a contribution rather than doing it myself.

phillipross commented 4 years ago

I tested PR #71 and nothing functionally broke, which is a good sign. And I looked at the test coverage and found that the JtsBinaryParser changes had test coverage but it appears JTSShape class has no test coverage at all. I merged the PR, and then merged an additional PR afterward to update the JTS dependencies to v1.16.1.

Now that there are official postgis docker images, continuous integration/release with automated testing will be implemented for this repo. A lot of long overdue changes are coming down the pipeline that should help reduce the burden of contributing to the project 🤞

phillipross commented 4 years ago

This change has been released in v2.4.0 release which will show up in maven central shortly. Also of note: this version of JTS has a minimum Java8 requirement and thus pushes the minimum required Java version of the PostGIS driver to also be Java8.