-
Notifications
You must be signed in to change notification settings - Fork 267
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
Handle import scope dependencies in dependencyManagement (multiple goals) (Fixes multiple issues #308, #309, #346, #395) #476
Conversation
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.
Hi @frode-carlson,
first of all, thanks for this contribution.
I have added remarks about formatting and it looks like you introduced tabs at a few places if I am not mistaken (before comment // handle normally declared dependencies
). And the blank lines before // handle import dependencies specially
It would be mighty helpful if you included an invoker test as well which checks at least some of your changes.
src/main/java/org/codehaus/mojo/versions/UseNextVersionsMojo.java
Outdated
Show resolved
Hide resolved
src/main/java/org/codehaus/mojo/versions/UseNextSnapshotsMojo.java
Outdated
Show resolved
Hide resolved
src/main/java/org/codehaus/mojo/versions/UseNextReleasesMojo.java
Outdated
Show resolved
Hide resolved
src/main/java/org/codehaus/mojo/versions/UseLatestVersionsMojo.java
Outdated
Show resolved
Hide resolved
src/main/java/org/codehaus/mojo/versions/UseLatestSnapshotsMojo.java
Outdated
Show resolved
Hide resolved
src/main/java/org/codehaus/mojo/versions/UseLatestReleasesMojo.java
Outdated
Show resolved
Hide resolved
src/main/java/org/codehaus/mojo/versions/UseDepVersionMojo.java
Outdated
Show resolved
Hide resolved
Fix tab issue Co-authored-by: Mirko Friedenhagen <mfriedenhagen@gmx.de>
add test resource file
@mfriedenhagen added test case and applied fixes for tabs |
@mfriedenhagen @hboutemy can anyone please review this again? |
This would be really handy |
Is there any chance of this PR getting merged? The documentation is very clear that the |
It doesn't? Yesterday I checked it and it did update those. I did check that with maven 3.2.5 as well as 3.6.3. Could you please share a test scenario where it doesn't update those dependencies? By that I understand a test pom.xml, not just a maven command line. |
Hi @ajarmoniuk thanks for your fast response! Here is a contrived <?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>com.example</groupId>
<artifactId>demo</artifactId>
<version>0.0.1-SNAPSHOT</version>
<name>demo</name>
<description>Demo project for Spring Boot</description>
<properties>
<java.version>11</java.version>
</properties>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-dependencies</artifactId>
<version>2.7.0</version> <!-- old version -->
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter</artifactId>
</dependency>
</dependencies>
</project> I have tested the above with both Maven 3.6.0 and 3.8.6 using command I would also note that the following <?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>com.example</groupId>
<artifactId>demo</artifactId>
<version>0.0.1-SNAPSHOT</version>
<name>demo</name>
<description>Demo project for Spring Boot</description>
<properties>
<java.version>11</java.version>
<spring.version>2.7.0</spring.version> <!-- old version -->
</properties>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-dependencies</artifactId>
<version>${spring.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter</artifactId>
</dependency>
</dependencies>
</project> |
Indeed, I was able to reproduce the issue using your POMs. Thank you. I will now update all mojos to include the fix for this issue. |
For some reason, I wasn't able to create a test using a bom in the |
This PR is stale because it has been open 365 days with no activity. Remove stale label or comment or this will be closed in 30 days. |
Fixes #308, Fixes #309, Fixes #346, Fixes #395 for following goals