rewrite icon indicating copy to clipboard operation
rewrite copied to clipboard

Added new recipe ChangeDependencyVersionValue

Open marcel-gepardec opened this issue 1 year ago • 4 comments

Recipe ChangeDependencyVersionValue

What's changed?

As discussed in another PR #4330 , I have now moved the change version part to a new recipe.

Nothing really changed from last PR, except the new recipe can also be called directly with new options. The first new one is versionLocation:

  • Changes dependency version right where you want it. The default is set to change it everywhere. But you can also specifically target DEPENDENCY, MANAGED_DEPENDENCY or PLUGIN_DEPENDENCY. It also doesn't matter if they are in a profile or not.

The second new option is newPropertyVersionValue:

  • If it is set than the property variable name is changed to it.

Example

new Recipe ChangeDependencyVersionValue:

---
type: specs.openrewrite.org/v1beta/recipe
name: com.example.openrewrite.migrate.NewJakartaPersistenceVersion
displayName: Change Maven managed dependency version with property example
recipeList:
  - org.openrewrite.maven.ChangeDependencyVersionValue:
      groupId: jakarta.persistence
      artifactId: jakarta.persistence-api
      newVersion: latest.release
      changePropertyVersionName: true
      newPropertyVersionName: test
      versionLocation: MANAGED_DEPENDENCY

Before:

                  ...
                  <properties>
                          <jakarta.persistence-api.version>1.2.0</jakarta.persistence-api.version>
                  </properties>
                  <dependencyManagement>
                      <dependencies>
                          <dependency>
                              <groupId>jakarta.persistence</groupId>
                              <artifactId>jakarta.persistence-api</artifactId>
                              <version>${jakarta.persistence-api.version}</version>
                          </dependency>
                      </dependencies>
                  </dependencyManagement>
                  <dependencies>
                      <dependency>
                           <groupId>jakarta.persistence</groupId>
                           <artifactId>jakarta.persistence-api</artifactId>
                           <version>1.2.0</version>
                      </dependency>
                  </dependencies>
                  ...

After:

                  ...
                  <properties>
                          <test>3.2.0</test>
                  </properties>
                  <dependencyManagement>
                      <dependencies>
                          <dependency>
                              <groupId>jakarta.persistence</groupId>
                              <artifactId>jakarta.persistence-api</artifactId>
                              <version>${test}</version>
                          </dependency>
                      </dependencies>
                  </dependencyManagement>
                  <dependencies>
                      <dependency>
                           <groupId>jakarta.persistence</groupId>
                           <artifactId>jakarta.persistence-api</artifactId>
                           <version>1.2.0</version>
                      </dependency>
                  </dependencies>
                  ...

marcel-gepardec avatar Jul 31 '24 14:07 marcel-gepardec

Thanks for splitting off these changes! I do see some overlap in the files changed and in

  • #4330

Since we squash merge; would it be possible to minimize the changes here such that we can only review what's related to the new recipe? Or did I misunderstand the change set seen here?

timtebeek avatar Jul 31 '24 20:07 timtebeek

Thanks for the feedback.

I only changed the code related to the new implementation of this recipe. I simplified the other recipes so there is no code duplication and I just moved the version change part to this recipe. I did not change anything else regarding this topic. So should I revert the changes I made to the other recipes?

That being said, in case we are not on the same page, every change set from the last PR #4330 is also included in this one, so you could just close the other PR #4330.

marcel-gepardec avatar Aug 01 '24 09:08 marcel-gepardec

That being said, in case we are not on the same page, every change set from the last PR #4330 is also included in this one, so you could just close the other PR #4330.

Ah that explains it then; I would have thought both PRs would separately be reviewed and merged, but we can also close #4330 then indeed.

timtebeek avatar Aug 01 '24 09:08 timtebeek

Yes, that would probably be the best.

marcel-gepardec avatar Aug 01 '24 10:08 marcel-gepardec

This PR seems to have amassed quite some conflicts since it was opened. I'm also not sure we'd want to support a new Boolean changePropertyVersionName option. Perhaps we should instead consider to simply rename properties that contain either the groupId or artifactId that is changed. It would mean no need to update recipes or defaults, and is likely to work out ok in most cases. Since that's significant departure from these efforts it might be best to close this PR for now. Thanks again for prompting this rethink!

timtebeek avatar Jan 21 '25 13:01 timtebeek