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

Allow PCT to use info from a bom file #202

Merged
merged 12 commits into from
Apr 20, 2020

Conversation

imonteroperez
Copy link
Contributor


File bomFile = new ClassPathResource("jenkins-bom.xml").getFile();
config.setBom(bomFile);
config.setIncludePlugins(includedPlugins);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test class needs config.setExternalMaven(<whatever-location>) i.e.: config.setExternalMaven(new File("/usr/bin/mvn")); but based on: #194 I let this without that config update.

@imonteroperez imonteroperez marked this pull request as ready for review October 21, 2019 06:56
@raul-arabaolaza
Copy link
Contributor

LGTM given we will wait for #194 to be merged to get a green build here :)

Copy link
Contributor

@raul-arabaolaza raul-arabaolaza left a comment

Choose a reason for hiding this comment

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

WE are missing the README update, but code LGTM

@imonteroperez
Copy link
Contributor Author

WE are missing the README update, but code LGTM

Updated README file on f9c66c2

data = scanBom(pluginGroupIds, "([^/.]+)[.][hj]pi");
} else {
data = config.getWar() == null ? extractUpdateCenterData(pluginGroupIds) : scanWAR(config.getWar(), pluginGroupIds, "WEB-INF/(?:optional-)?plugins/([^/.]+)[.][hj]pi");
}
Copy link
Member

Choose a reason for hiding this comment

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

Interesting but will not fix the problem in #228, since we would still be contacting the UC.

This PR could solve the problem in a different way, by extracting groupIds from the BOM including for detached plugins. Perhaps it is already effectively doing that, or we can assume that the BOM manages versions for all detached plugins of interest, in which case this PR could simply be extended to skip extractUpdateCenterData in case -bom is passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your comments @jglick in this PR we are assuming that BOM manages versions for all detached plugins too, anyway I added here the skip of extractUpdateCenterData in case bom file is passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants