Skip to content
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.

Sign up for GitHub

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

Jump to bottom

RemoveRedundantDependencyVersions - restore ability to remove newer version pins #4138

Closed
nmck257 opened this issue Apr 16, 2024 · 1 comment · Fixed by #4148
Closed

RemoveRedundantDependencyVersions - restore ability to remove newer version pins #4138

nmck257 opened this issue Apr 16, 2024 · 1 comment · Fixed by #4148
Assignees
@nmck257
Labels
recipe Requested Recipe

Comments

@nmck257
Copy link
Collaborator

nmck257 commented Apr 16, 2024

What problem are you trying to solve?

Add an option for the functionality removed in this commit:
687fbb5
(from @sambsnyd )

What precondition(s) should be checked before applying this recipe?

N/A

Describe the situation before applying the recipe

              <project>
                  <modelVersion>4.0.0</modelVersion>
                  <groupId>org.sample</groupId>
                  <artifactId>sample</artifactId>
                  <version>1.0.0</version>
                  <parent>
                      <groupId>org.springframework.boot</groupId>
                      <artifactId>spring-boot-starter-parent</artifactId>
                      <version>2.7.18</version>
                      <relativePath/>
                  </parent>
                  <build>
                      <plugins>
                          <plugin>
                              <groupId>pl.project13.maven</groupId>
                              <artifactId>git-commit-id-plugin</artifactId>
                              <!-- version in pom is 4.9.10, pin is newer-->
                              <version>4.9.11</version>
                          </plugin>
                      </plugins>
                  </build>
              </project>

Describe the situation after applying the recipe

              <project>
                  <modelVersion>4.0.0</modelVersion>
                  <groupId>org.sample</groupId>
                  <artifactId>sample</artifactId>
                  <version>1.0.0</version>
                  <parent>
                      <groupId>org.springframework.boot</groupId>
                      <artifactId>spring-boot-starter-parent</artifactId>
                      <version>2.7.18</version>
                      <relativePath/>
                  </parent>
                  <build>
                      <plugins>
                          <plugin>
                              <groupId>pl.project13.maven</groupId>
                              <artifactId>git-commit-id-plugin</artifactId>
                          </plugin>
                      </plugins>
                  </build>
              </project>

Have you considered any alternatives or workarounds?

Any additional context

This (removed) behavior can be useful if you want to strictly rely on parent-provided versions, and implicitly "distrust" the validity of existing pinned versions (or anticipate that they will become a problem eventually as the parent version increases)

Are you interested in contributing this recipe to OpenRewrite?

Yes -- mainly opening this issue first in case anyone has design input.

The least-intrusive implementation would be a new, additional flag, such as removeNewerVersions, which works in conjunction with onlyIfVersionsMatch. But the naming/usage starts to get fuzzy -- what if someone sets onlyIfVersionsMatch true and also removeNewerVersions true? We can validate those cases out, but, it's unintuitive.

Could also argue that onlyIfVersionsMatch is now an inaccurate name, since on false, it does not remove non-matching newer versions.

Replacing this field with some kind of removalStrategy enum could be nice, with values like matching, older, any, but replacing an arg like that can be disruptive for existing usage, and naming the new arg also takes some good thought (I don't love the value names I just suggested, eg).

Eager for any opinions
@nmck257 nmck257 added the recipe Requested Recipe label Apr 16, 2024
@nmck257
Copy link
Collaborator Author

nmck257 commented Apr 16, 2024 •
edited

maybe replace onlyIfVersionsMatch with onlyIfManagedVersionIs, with the value being a comparator symbol? eg =, <, >=?
and maybe * for "remove any relevant version regardless of value"?

default would be = to match default behavior of existing arg; could also interpret any number of = characters all the same way in case someone has muscle memory for == or ===

or maybe avoiding special characters is desirable for yaml, and instead could do eq, lt, gte, etc

@nmck257 nmck257 mentioned this issue Apr 23, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nmck257 nmck257

Labels
recipe Requested Recipe
Projects
OpenRewrite
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

RemoveRedundantDependencyVersions - adding onlyIfManagedVersionIs o… openrewrite/rewrite
1 participant
@nmck257