Do not override version property placeholder in UpgradeDependencyVersion #4579
Conversation
…lue contains the version, I added an extra check getting the value directly from the pom
rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java
Outdated
Show resolved
Hide resolved
timtebeek
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java
Outdated
Show resolved
Hide resolved
…dencyVersion.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java
Outdated
Show resolved
Hide resolved
…dencyVersion.java Co-authored-by: Tim te Beek <timtebeek@gmail.com>
| void upgradePropertyAdditionalCheck() { | ||
| rewriteRun( | ||
| spec -> spec.recipe(new UpgradeDependencyVersion("org.springframework", "*", "5.2.x", "", | ||
| false, null)).expectedCyclesThatMakeChanges(2), |
There was a problem hiding this comment.
Can we check once more whether we can reduce the amount of cycles to 1?
Enforcing that all recipes complete in 1 cycle is something that's being looked at
| <name>example</name> | ||
| <description>Demo project for Spring Boot</description> | ||
| <properties> | ||
| <spring.boot.version>2.2.13.RELEASE</spring.boot.version> |
There was a problem hiding this comment.
This version property should be updated too, right?
There was a problem hiding this comment.
Indeed in the ticket it expects to upgade also the boot dependency, but this should be done in the rewrite-spring repository in this file resources/META-INF/rewrite/spring-framework-53.yml file
In the rewrite-spring repository the recipe is invoked with the following properties
groupId: org.springframework
artifactId: "*"
newVersion: 5.3.x
I used the same for this test.
To upgrade the boot dependency, something like this should be added:
- org.openrewrite.java.dependencies.UpgradeDependencyVersion:
groupId: org.springframework.boot
artifactId: "*"
newVersion: 2.x
There was a problem hiding this comment.
My bad, didn't read the group id properly, you are right :)
What's changed?
bug-474: added relevant test, in some cases the provided requested value contains the version, I added an extra check getting the value directly from the pom
What's your motivation?
The provided requested versions was not always correct
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
@Laurens-W
Have you considered any alternatives or workarounds?
Any additional context
This solves half the issue, it stops the placeholder from being replaced from the version.
The spring boot version is still not upgraded, but this is because of the way that the upgrade call is done
with org.springframework and not with org.springframework.boot
Checklist