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.

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 postgis plugin with support for JTS data types #2072

Closed
wants to merge 1 commit into from

Conversation

bchapuis
Copy link
Contributor

@bchapuis bchapuis commented Jul 22, 2022

This PR adds support for postgis and map geospatial data types to JTS data types.

As this is my first contribution to JDBI, I would love to get feedbacks:

  • Does the plugin fits in the postgres module or should I create a new postgis module?
  • Should the factory and mapper class be moved at the level of the package?
  • How to test postgis without testcontainers as the setup is slighly different from postgres?
  • Anything else that raises eyebrows.

Thanks a lot for maintaining JDBI.

@hgschmie
Copy link
Contributor

Hi @bchapuis !

Thank you for contributing to JDBI! Your code is generally sound but you did a lot of work that was not necessary. :-)

I cleaned up the code in #2074 (and also made the test work and pass with our normal test framework). Please review and let me know if that works for your use case.

Support for Postgres PostGIS extension.

Co-Authored-By: Henning Schmiedehausen <henning@schmiedehausen.org>
@bchapuis
Copy link
Contributor Author

bchapuis commented Jul 24, 2022

@hgschmie Thanks a lot for your feedback, it simplifies things a lot! Do you mind if we merge this PR instead of #2074? I cherry picked your changes, added you as a co-author and force pushed. I'd prefer to appear in the commit history than in the git comment. ;)

@sonarcloud
Copy link

sonarcloud bot commented Jul 24, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@hgschmie
Copy link
Contributor

merged as #2074

@hgschmie hgschmie closed this Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants