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

Update jclouds, picking up Guice from core #215

Merged
merged 10 commits into from
Nov 17, 2021
Merged

Conversation

jglick
Copy link
Member

@jglick jglick commented Jun 14, 2021

Derived from #189. Follows up #244 to pick up jenkinsci/jenkins#5858. #226 (comment) tracks a further jclouds upgrade. Possibly solves JENKINS-65824.

@@ -222,6 +223,11 @@ public String putBlob(String containerName, Blob blob) throws IOException {
return null;
}

@Override
Copy link
Member Author

Choose a reason for hiding this comment

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

@jglick jglick requested a review from jtnord June 14, 2021 20:33
@jglick
Copy link
Member Author

jglick commented Jun 14, 2021

@grochoge can you please download this build, install it via the Advanced tab of the plugin manager, and verify whether it indeed fixes JENKINS-65824 for you?

pom.xml Outdated Show resolved Hide resolved
@jtnord

This comment has been minimized.

@jglick

This comment has been minimized.

@jglick

This comment has been minimized.

@jglick

This comment has been minimized.

@jglick jglick changed the title Update jclouds Update jclouds, picking up Guice from core Nov 17, 2021
@jglick jglick requested review from jtnord, basil and Vlatombe November 17, 2021 21:18
@@ -16,7 +16,7 @@
<properties>
<revision>1.18</revision>
<changelist>-SNAPSHOT</changelist>
<jenkins.version>2.320</jenkins.version>
<jenkins.version>2.321</jenkins.version>
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that the bom is still old. #242 tracks problems with that.

Comment on lines +266 to +278
<version>5.0.1</version>
</dependency>
</dependencies>
</dependencyManagement>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<configuration>
<rules combine.children="append">
<requireSameVersions>
Copy link
Member

Choose a reason for hiding this comment

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

Should this be in core if it is tightly coupled to guice version (at least until guice gets removed from core 🦄)

Copy link
Member Author

Choose a reason for hiding this comment

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

Should this be in core if it is tightly coupled to guice version

It could be, though I suppose core does not use it; or it could be added to the core BOM.

at least until guice gets removed from core

Or I can remove jclouds from this plugin: #215 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

It could be, though I suppose core does not use it; or it could be added to the core BOM.

if it was just in the BOM then it can constantly drift (core gets a new Guice, this plugin potentially breaks).

Or I can remove jclouds from this plugin: #215 (comment)

probably better.

Copy link
Member Author

Choose a reason for hiding this comment

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

core gets a new Guice, this plugin potentially breaks

Potentially, though the test failure I saw was about core bundling Guice 5 and this plugin bundling Guice Assisted Inject 4; I hope breaking changes are limited to major versions which should not be too common. The Enforcer rule will also alert us to problems if are updating the core dep for other reasons. (Might also work in PCT, not sure.)

@jglick jglick merged commit 376f8ae into jenkinsci:master Nov 17, 2021
@jglick jglick deleted the aws-s3 branch November 17, 2021 22:42
@jglick jglick mentioned this pull request Nov 17, 2021
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.

3 participants