-
Notifications
You must be signed in to change notification settings - Fork 2
Swap to shaded guava in common and core modules #8
Conversation
| } | ||
|
|
||
| dependencies { | ||
| exclude(dependency('com.github.stephenc.findbugs:findbugs-annotations')) |
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.
Can you move these up into the section starting on line 115 so they are all in the same place?
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'm not sure this can be moved, I think this exclude(dependency(..)) thing is part of the shadowJar plugin, so it needs to happen here rather than in the module dependencies section
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.
Yes, you're right, sorry, I missed that.
| } | ||
|
|
||
| project(':iceberg-common') {} | ||
| project(':iceberg-common') { |
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.
Do you not also need to do this for iceberg-arrow? Or does it get the relocate-external-dependencies transitively?
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.
yeah, it can get them transitively here
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.
OK, I don't like the fact that this is how it works, I prefer declaring what I know I need but let's not change it for now.
| } | ||
|
|
||
| dependencies { | ||
| exclude(dependency('com.github.stephenc.findbugs:findbugs-annotations')) |
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.
Yes, you're right, sorry, I missed that.
| } | ||
|
|
||
| project(':iceberg-common') {} | ||
| project(':iceberg-common') { |
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.
OK, I don't like the fact that this is how it works, I prefer declaring what I know I need but let's not change it for now.
Had to exclude a few dependencies from the
relocate-external-dependenciesbecause of runtime class uniqueness issues as it the module pulls in a few dependencies from the shared project-wide dependencies