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

Add a new infra.prepareToPublishIncrementals() method to optimize the resource usage #111

Merged

Conversation

jglick
Copy link
Contributor

@jglick jglick commented Aug 28, 2019

Copy link

@jeffret-b jeffret-b left a comment

Choose a reason for hiding this comment

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

Makes sense to me, but I'm not familiar with this file and how it's used.

*/
void prepareToPublishIncrementals() {
// MINSTALL-126 would make this easier by letting us publish to a different directory to begin with:
String m2repo = sh script: 'mvn -Dset.changelist -Dexpression=settings.localRepository -q -DforceStdout help:evaluate', returnStdout: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not infra.runMaven() ? The mirror will not be used for plugin download

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You cannot use runMaven for this purpose: returnStdout.

This could call retrieveMavenSettingsFile again (with a sufficiently distinctive path), though it seems like overkill. This would typically be run after the main build, so the only additional plugins that might be downloaded would be maven-help-plugin I think.

/**
* When appropriate, publish artifacts from the current build to the Incrementals repository.
* Call at the end of the build, outside any node, when #prepareToPublishIncrementals may have been called previously.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it a strong requirement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What? To call outside node? Well nothing will break if you call it inside node, but you will be wasting executor minutes.

Copy link
Contributor

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

Yes please.

@oleg-nenashev oleg-nenashev changed the title infra.prepareToPublishIncrementals() Add a new infra.prepareToPublishIncrementals() method to optimize the resource use Aug 31, 2019
@oleg-nenashev oleg-nenashev changed the title Add a new infra.prepareToPublishIncrementals() method to optimize the resource use Add a new infra.prepareToPublishIncrementals() method to optimize the resource usage Aug 31, 2019
@oleg-nenashev oleg-nenashev merged commit 76aad23 into jenkins-infra:master Aug 31, 2019
@jglick jglick deleted the prepareToPublishIncrementals branch September 3, 2019 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants