Skip to content

Upgrade to JTS 1.17.0. #188

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

Merged
merged 3 commits into from
Jul 8, 2020
Merged

Upgrade to JTS 1.17.0. #188

merged 3 commits into from
Jul 8, 2020

Conversation

jnh5y
Copy link
Contributor

@jnh5y jnh5y commented Jul 3, 2020

Signed-off-by: Jim Hughes [email protected]

Signed-off-by: Jim Hughes <[email protected]>
@dsmiley
Copy link
Contributor

dsmiley commented Jul 4, 2020

Thanks! Can you update CHANGES.md as well please? In like-kind to past entries.

@jnh5y
Copy link
Contributor Author

jnh5y commented Jul 6, 2020

@dsmiley of course. Are the numbers, the PR number or the issue number? (I assumed the former.)

Any more that I can say for the update?

@codecov-commenter
Copy link

Codecov Report

Merging #188 into master will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #188      +/-   ##
============================================
- Coverage     75.84%   75.76%   -0.08%     
+ Complexity     1108     1105       -3     
============================================
  Files            57       57              
  Lines          3817     3817              
  Branches        776      774       -2     
============================================
- Hits           2895     2892       -3     
  Misses          643      643              
- Partials        279      282       +3     
Impacted Files Coverage Δ Complexity Δ
...iontech/spatial4j/shape/impl/ShapeFactoryImpl.java 79.12% <0.00%> (-2.20%) 37.00% <0.00%> (-2.00%)
.../locationtech/spatial4j/shape/jts/JtsGeometry.java 77.41% <0.00%> (-0.36%) 79.00% <0.00%> (-1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63b3183...d3f6d9a. Read the comment docs.

@dsmiley dsmiley merged commit 79e5f22 into locationtech:master Jul 8, 2020
@dsmiley
Copy link
Contributor

dsmiley commented Aug 1, 2020

JTS 1.17 included a change to upgrade Java 1.7 to 1.8 -- https://github.com/locationtech/jts/releases/tag/1.17.0 and this means Spatial4j's builds must now require at least 1.8 as well, but it can keep javac source & target options to 1.7. Users can continue to run Spatial4j with Java 1.7 but will need to upgrade to use the JTS we specify if they want to use JTS (it's an optional dependency). Strangely, TravisCl hasn't been pinging me about the failure but I did seek it out:
https://travis-ci.org/github/locationtech/spatial4j/builds/706041451
I wish it showed up here in the PR. Hmm; something to look in to.

@jnh5y
Copy link
Contributor Author

jnh5y commented Aug 3, 2020

Sorry for that oversight on my part. I didn't realized that Spatial4J was still supporting Java 7 and that JTS had moved up to requiring Java 8 in 1.17.0.

I don't see the results from the Git WebHook with Travis CI here. Maybe those need to configured differently? I know for GeoMesa that the GitHub webhooks will show green check or red X's for a number of criteria. (Let me know if I can help with any of that.)

@dsmiley
Copy link
Contributor

dsmiley commented Aug 5, 2020

Yes please, I would appreciate help hooking up Travis CI to the PR. Do I just need to RTFM (URL please)?

@jnh5y
Copy link
Contributor Author

jnh5y commented Aug 5, 2020

We'll likely have to email the Eclipse webmaster to ask about some GitHub settings.

After thinking about it, would it be worth considering GitHub Actions instead? (If you are willing to try them, I may be able to find a few minutes to gin up an example of GH Actions that'd run the builds you are using Travis for.)

If we go that route, is Java 7 support still important to you? (If so, that's fine. That said, I'd love to hear a reason why.)

@dsmiley
Copy link
Contributor

dsmiley commented Aug 5, 2020

No Java 7 is too old, but I still -source & -target at the complication step to 7. Note that since I added in JDK 11 to Travis very recently, there seems to be some test maintenance for me to do in order to make tests work in JDK 11. Work for me to do soon.

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