-
Notifications
You must be signed in to change notification settings - Fork 376
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
UpgradePluginVersion
-- ability to upgrade maven plugin version
#579
Conversation
05a03f6
to
17f49ad
Compare
UpgradePluginVersion
-- ability to upgrade maven plugin version
17f49ad
to
ca14620
Compare
3d3587c
to
105dc35
Compare
105dc35
to
34c0322
Compare
e45173b
to
535b0d5
Compare
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.
Looking good Aaron, my only suggestion would be to move the "maybeChangeVersion" logic to an applicability test (a visitor that marks up the tree if a change will be made).
} | ||
|
||
@Override | ||
public Maven visitMaven(Maven maven, ExecutionContext ctx) { |
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.
Thinking we can move the logic for finding if we are going to make a change to the applicability test.
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.
I implemented an applicability test which simply checks for the existance of the desired plugin to upgrade. I fully expect for this entire visitor to be cleaned up to be more efficient, though the API for it's usage (options
) should be stable, and it does work (assuming you aren't trying to get a version from pluginManagement, etc.). But, it does work. Forgive me for this, but I'm going to merge it knowing full-well it could use more love 😬
b604b38
to
3d94cf4
Compare
3d94cf4
to
04e45da
Compare
…ings (#579) * Fix issue 568, trailing semi-colon on property accessor without surroundings * polish * clean up warning
see: #570
see downstream issue which prompted this: openrewrite/rewrite-quarkus#6
see: #585
This code looks wonky, I know. It's loosely based on upgradePluginDependency. Would be enhanced by virtue of having
pom
andmaven
model supports pluginManagementIn order to enable upgrading a maven plugin to a different version, include a recipe for specifying a
version
to upgrade a plugin to a later version.