-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix Guava shading in jdbc module #399
Conversation
@@ -16,6 +16,18 @@ | |||
|
|||
<dependencies> | |||
<dependency> | |||
<groupId>junit</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it here, otherwise Maven would add it to every module as compile
dependency. However, modules should use it from testcontainers
dependency as a transitive.
@@ -22,8 +22,53 @@ | |||
<groupId>com.google.guava</groupId> | |||
<artifactId>guava</artifactId> | |||
<version>18.0</version> | |||
<scope>provided</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn!
<include>com.google.guava:*</include> | ||
</includes> | ||
</artifactSet> | ||
<promoteTransitiveDependencies>false</promoteTransitiveDependencies> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This flag must be set to false
to avoid including testcontainers
' dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine for a quick fix. Still - I think we could probably quite easily remove the use of Guava altogether. Maybe something to do when we have a bit more time though.
1.4.0 introduced a bug in
jdbc
module. The shading was removed from it in #390, and Guava was inprovided
scope.