Skip to content

Move recipe Replace Libraries With Api Plugin#791

Merged
jonesbusy merged 2 commits into
jenkins-infra:mainfrom
CodexRaunak:custom-recipe-ReplaceLibrariesWithApiPlugin
Feb 12, 2025
Merged

Move recipe Replace Libraries With Api Plugin#791
jonesbusy merged 2 commits into
jenkins-infra:mainfrom
CodexRaunak:custom-recipe-ReplaceLibrariesWithApiPlugin

Conversation

@CodexRaunak
Copy link
Copy Markdown
Contributor

Issue #637
Moved the Recipe ReplaceLibrariesWithApiPlugin in the plugin-modernizer-tool recipes. Exclusions are not added for libraries that the API plugin handles, as the API plugin is designed to bring those dependencies to the consumer.

Testing done

Added the same pom.xml as a test case where the duplicate tag issue was occurring.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@CodexRaunak
Copy link
Copy Markdown
Contributor Author

CodexRaunak commented Feb 10, 2025

Need a review on this,
I added the same pom.xml as a test case where it was adding duplicate tags #637. The exclusions are not added now at all in json-api. But its skipping the exclusions for every library that is a part of API plugin. I am not sure we want that? Also it needs more tests.

if (isApiProvidedLibrary(groupId, artifactId)) {
                                   continue; // Skip exclusions if the library is already part of the API plugin
 }

Thank you

<dependency>
<groupId>io.jenkins.tools.bom</groupId>
<artifactId>bom-${jenkins.baseline}.x</artifactId>
<version>4051.v78dce3ce8b_d6</version>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's be careful with target bom version since it's moving every week.

Better to use Settings.getBomVersion() where possible

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we don't need it here as we are not upgrading the jenkins version and not tracking the weekly release line.

@jonesbusy
Copy link
Copy Markdown
Collaborator

Hi,

Thanks!

I think we should not exclude anything comming from an API plugin.

Genearally they are used with a BOM, so we should not have dependency convergence issue

I think the situation is much better now than it was originally

@CodexRaunak CodexRaunak force-pushed the custom-recipe-ReplaceLibrariesWithApiPlugin branch from 598de2a to 540c08e Compare February 11, 2025 21:06
@CodexRaunak
Copy link
Copy Markdown
Contributor Author

CodexRaunak commented Feb 11, 2025

The recipe is not excluding anything from the API plugin, added unit tests for the same

@jonesbusy jonesbusy added enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted chore and removed enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted labels Feb 12, 2025
@jonesbusy jonesbusy merged commit 80821a4 into jenkins-infra:main Feb 12, 2025
@CodexRaunak CodexRaunak deleted the custom-recipe-ReplaceLibrariesWithApiPlugin branch February 12, 2025 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants