-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 dependency management and junit4 dependency to parent pom #1912
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,15 +15,6 @@ | |
<properties> | ||
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> | ||
<slf4j.version>1.7.26</slf4j.version> | ||
<log4j.version>1.2.17</log4j.version> | ||
<commons-compress.version>1.18</commons-compress.version> | ||
|
||
<!-- for the correct jackson and guava versions see https://github.com/dropwizard/dropwizard/blob/release/1.3.x/dropwizard-bom/pom.xml --> | ||
<!-- make also sure that the dropwizard version is working with 1.3.5 of dropwizard-configurable-assets-bundle used in web --> | ||
<dropwizard.version>1.3.12</dropwizard.version> | ||
<jackson.version>2.9.9</jackson.version> | ||
<guava.version>24.1.1-jre</guava.version> | ||
|
||
<maven.compiler.target>1.8</maven.compiler.target> | ||
|
||
|
@@ -89,6 +80,69 @@ | |
<module>web</module> | ||
<module>client-hc</module> | ||
</modules> | ||
<dependencyManagement> | ||
<dependencies> | ||
<!-- provides compatible versions for jackson, guava, slf4j etc. --> | ||
<dependency> | ||
<groupId>io.dropwizard</groupId> | ||
<artifactId>dropwizard-bom</artifactId> | ||
<!-- make sure the dropwizard version is compatible to the below dropwizard-configurable-assets-bundle version --> | ||
<version>1.3.12</version> | ||
<type>pom</type> | ||
<scope>import</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>io.dropwizard-bundles</groupId> | ||
<artifactId>dropwizard-configurable-assets-bundle</artifactId> | ||
<version>1.3.5</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>com.graphhopper.external</groupId> | ||
<artifactId>jackson-datatype-jts</artifactId> | ||
<version>0.12-2.5-1</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>com.carrotsearch</groupId> | ||
<artifactId>hppc</artifactId> | ||
<version>0.8.1</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.locationtech.jts</groupId> | ||
<artifactId>jts-core</artifactId> | ||
<version>1.15.1</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.apache.commons</groupId> | ||
<artifactId>commons-compress</artifactId> | ||
<version>1.18</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>log4j</groupId> | ||
<artifactId>log4j</artifactId> | ||
<version>1.2.17</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.slf4j</groupId> | ||
<artifactId>slf4j-log4j12</artifactId> | ||
<!-- make sure this matches the slf4j version in the dropwizard bom --> | ||
<version>1.7.26</version> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unfortunately this logging bridge is not declared in the dropwizard bom, while for example slf4j-api is declared there (not sure why) |
||
<scope>test</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>junit</groupId> | ||
<artifactId>junit</artifactId> | ||
<version>4.12</version> | ||
</dependency> | ||
</dependencies> | ||
</dependencyManagement> | ||
|
||
<dependencies> | ||
<dependency> | ||
<groupId>junit</groupId> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this here with the downside that all child projects have this dependency automatically now. Then again junit is used by all children and its only test scope too. With junit 5 this will be multiple dependencies, which otherwise we would have to repeat in every child pom. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, I did the exact opposite a while ago. I don't oppose at the moment, but let's remember that this is only for convenience: There ist absolutely zero dependency between the test frameworks each submodule uses (each one can use its own), so in a way this is an artificial restriction to save typing. Totally okay as long as one doesn't forget. :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes its code re-use via inheritance in a way, use with care... If you want we can of course go back to declaring the dependency in the submodules for the reason you mention. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't want to be pedantic but a submodule does not inherit dependencies from a parents dependencyManagement section and is still free to override the dependency versions in its own pom. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but the junit dependency is an actual dependency (not dependencyManagement). I think we all agree that using the dependencyManagement in the parent pom makes sense (?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sorry, my bad.
Absolutely |
||
<artifactId>junit</artifactId> | ||
<scope>test</scope> | ||
</dependency> | ||
</dependencies> | ||
<build> | ||
<plugins> | ||
<plugin> | ||
|
@@ -254,7 +308,7 @@ | |
<artifactId>maven-javadoc-plugin</artifactId> | ||
<version>3.1.0</version> | ||
<configuration> | ||
<quiet>true</quiet> | ||
<quiet>true</quiet> | ||
</configuration> | ||
<executions> | ||
<execution> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason you added this link to the bom file, but did not include the bom file here? It provides the correct version for jackson, guava and many more, see below (maybe too many and it would be more readable to specify these versions explicitly in our dependency management section instead?)