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.

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

Generalize property handling in Maven recipes? #4527

Open
gsmet opened this issue Sep 26, 2024 · 2 comments
Open

Generalize property handling in Maven recipes? #4527

gsmet opened this issue Sep 26, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@gsmet
Copy link
Contributor

gsmet commented Sep 26, 2024

Hi,

We have been very happy with the property handling in UpgradeDependencyVersion: when a version is a property, the property is updated instead of the original version tag.
Which is awesome as a lot of versions are properties.

All good but... in Quarkus, we are using properties in generated projects also for the groupId/artifactId of the Quarkus BOM (and it's something that is important to us as we have several "Platforms" with different coordinates and we need to enforce the Platform consistently to the BOM and the Quarkus Maven plugin, thus why we use properties).

While working on a PR to handle changing the Platform entirely (i.e. including the groupId/artifactId, until now we were just updating the version), I stumbled upon the fact that this nice property handling is actually not generalized to the Maven recipes.

For instance, there is nothing to handle it in ChangeDependencyGroupIdAndArtifactId or ChangeManagedDependencyGroupIdAndArtifactId (unless I missed something but the code seems in line with the behavior I'm experiencing).

I'm not sure exactly why this is handled only in a very specific case. I'm also not completely sure it should be generalized everywhere but not handling it for the groupId/artifactId is actually quite blocking for us. And to be fair, the version is not handled either in ChangeDependencyGroupIdAndArtifactId and ChangeManagedDependencyGroupIdAndArtifactId so it's not very consistent.

I was wondering if it's something you would consider? If you had some advice on how to fix it? I could probably dedicate some time to it as it's in our way but I definitely need some guidance. Also, I might not be able to generalize it everywhere in the time I would be able to dedicate to it but I could at least put the infrastructure in place and fix the cases that are in our way, opening the gate for more in the future if someone needs it elsewhere.

Thoughts?

@gsmet gsmet added the enhancement New feature or request label Sep 26, 2024
@timtebeek
Copy link
Contributor

Hi @gsmet ! Thanks for the additional context! I think the Jenkins folks have similar requirements, as seen on this existing PR:

Haven't had time to review that just yet, and have a few weeks of travel ahead, but property resolve is certainly something that we could support better, but fitting in that work might take us a while. I understand you're similarly busy over there, so for now we'll track support here

@gsmet
Copy link
Contributor Author

gsmet commented Sep 27, 2024

I think it's actually a bit orthogonal. Because AFAICS, OpenRewrite actually resolves my properties as the correct dependencies are adjusted. What it does not is rewrite the value of the property instead of rewriting the <artifactId> tag.

I'll draft something soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

No branches or pull requests

2 participants