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

Resove dependencies bom with properties on ChangeParentPom #4735

Merged

Conversation

jonesbusy
Copy link
Contributor

@jonesbusy jonesbusy commented Dec 1, 2024

This just demonstrate an issue we are facing with jenkins modernization.

The ChangeParentPom recipe wrongly add a dependency into dependency management when it should not

Such dependency is still managed by the bom.

    +            <dependency>
    +                <groupId>org.jenkins-ci.plugins</groupId>
    +                <artifactId>scm-api</artifactId>
    +                <version>698.v8e3b_c788f0a_6</version>
    +            </dependency>

I think it cause issue because the ${jenkins.baseline} on the artifact id.

But I don't know how and the best way to fix it

Any help would be appreciated

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@jonesbusy jonesbusy force-pushed the bugfix/upgrade-parent-keep-dependencies branch from e3583ff to f47372f Compare December 1, 2024 12:59
@jonesbusy jonesbusy changed the title Add test to demonstrate issue with ChangeParentPom adding dependency management Resove dependencies bom with properties on ChangeParentPom Dec 1, 2024
@timtebeek timtebeek added the bug Something isn't working label Dec 1, 2024
Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for diving in here @jonesbusy ; in your commit message you've marked this as a tentative fix; should we hold off on a merge just yet?

@jonesbusy
Copy link
Contributor Author

Yes 2nd commit is the fix. I had some failure locally, but was probably related to my environment.

CI looks good so we can move forward

@timtebeek timtebeek self-requested a review December 1, 2024 18:10
@timtebeek timtebeek merged commit 3b40168 into openrewrite:main Dec 1, 2024
2 checks passed
@jonesbusy jonesbusy deleted the bugfix/upgrade-parent-keep-dependencies branch December 1, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants