Clean out querydsl-sql dependencies #814

Closed
balazs-zsoldos opened this Issue Jun 19, 2014 · 3 comments

Comments

Projects
None yet
3 participants
@balazs-zsoldos
Contributor

balazs-zsoldos commented Jun 19, 2014

This issue contains the following changes:

  • make querydsl-spatial optional (in my opinion sql-spatial should be a separate module)
  • remove com.vividsolutions:jts dependency: I do not know why it is there. All tests run well and compilation ok if I remove it
  • replace javax.validation to a version that contains OSGi manifest headers
  • replace javax.inject to the servicemix bundle as it contains OSGi headers
  • make javax.inject optional
  • make postgresql dependency optional
  • make postgis dependency optional
  • make oracle dependencies optional

Notes:

The last three dependencies came in only because of spatial (as much as I see). I would consider creating an own querydsl-sql-spatial module for it to be able to handle its dependencies and improvements separately.

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Jun 19, 2014

Member

remove com.vividsolutions:jts dependency: I do not know why it is there. All tests run well and compilation ok if I remove it.

That is because it is a transitive dependency, but because we depend on some objects in the artifact, we declare it as a direct dependency, so I recommend putting it back.

As for the database dependencies, optional is good in my opinion, but the version numbers should reflect at build time our compatible dependency.

I will comment more when I have time

Member

Shredder121 commented Jun 19, 2014

remove com.vividsolutions:jts dependency: I do not know why it is there. All tests run well and compilation ok if I remove it.

That is because it is a transitive dependency, but because we depend on some objects in the artifact, we declare it as a direct dependency, so I recommend putting it back.

As for the database dependencies, optional is good in my opinion, but the version numbers should reflect at build time our compatible dependency.

I will comment more when I have time

@balazs-zsoldos

This comment has been minimized.

Show comment
Hide comment
@balazs-zsoldos

balazs-zsoldos Jun 19, 2014

Contributor

That is because it is a transitive dependency

I see. I put it back. If you check the pull request, it is there again.

Another reason to move spatial support out from querydsl-sql to a separate querydsl-sql-spatial module ;-)

Contributor

balazs-zsoldos commented Jun 19, 2014

That is because it is a transitive dependency

I see. I put it back. If you check the pull request, it is there again.

Another reason to move spatial support out from querydsl-sql to a separate querydsl-sql-spatial module ;-)

@timowest timowest added the fixed label Jun 20, 2014

@timowest timowest modified the milestone: 3.4.1 Jun 20, 2014

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jun 29, 2014

Member

Released in 3.4.1

Member

timowest commented Jun 29, 2014

Released in 3.4.1

@timowest timowest closed this Jun 29, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment