-
Notifications
You must be signed in to change notification settings - Fork 36
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
8.7.0: fix dependencyManagement section usage in maven pom.xml parser #578
Conversation
173804c
to
b23c235
Compare
<groupId>com.google.guava</groupId> | ||
<artifactId>guava</artifactId> | ||
<version>${guava.version}</version> | ||
</dependency> |
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.
here I'm moving the <version>
field for guava from <dependencies>
into <dependencyManagement>
to assert that we'll use the latter as fallback.
(note that this still uses the interpolation string ${guava.version}
too, so this is also asserting that any <version>
in <dependencyManagement>
also benefits from property interpolation)
@@ -93,9 +94,6 @@ | |||
{ name: "io.libraries:recursive", requirement: "${recursive.test}", type: "runtime" }, | |||
{ name: "io.libraries:optional", requirement: "${optional.test}", type: "runtime", optional: true }, | |||
{ name: "io.libraries:not-optional", requirement: "${not-optional.test}", type: "runtime", optional: false }, | |||
# From dependencyManagement section | |||
{ name: "org.apache.ant:ant", requirement: "1.9.2", type: "runtime" }, | |||
{ name: "commons-lang:commons-lang",requirement: "2.6", type: "runtime" } |
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.
these were only found in the <dependencyManagement>
section but not in <dependencies>
, so Maven wouldn't add these to the project
@@ -1,3 +1,3 @@ | |||
module Bibliothecary | |||
VERSION = "8.6.5" | |||
VERSION = "8.7.0" |
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.
this change doesn't break compatibility, but it could mean fewer dependencies from maven projects, so I'm considering this a bugfix / change in behavior, hence the minor version bump.
In version 6.6.0 we added support for deps declared in the
<dependencyManagement>
section. This section is only for dependency configuration though, not for explicitly importing deps into the project.This change reverts that behavior so we no longer import those deps, but we'll use the information (version/scope) in there as fallbacks for the actual deps in
<dependencies>
section instead.#451