Skip to content

Conversation

@rdblue
Copy link
Contributor

@rdblue rdblue commented May 29, 2020

This excludes Guava from compileClasspath so that Iceberg cannot use the classes directly, but tests will still pass because the testRuntime classpath does have transitive Guava classes.

@rdblue
Copy link
Contributor Author

rdblue commented May 29, 2020

FYI @rdsr, @shardulm94, @massdosage.

This also fixes the classes added in #1064 that were not updated by #1076.

@shardulm94
Copy link
Contributor

+1 LGTM

testCompile.extendsFrom compileOnly

compileClasspath {
// do not exclude Guava so the bundle project can reference classes.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: slight rewording? . Exclude guava from iceberg modules except ... ?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@rdblue rdblue merged commit 201f8bf into apache:master May 29, 2020
@rdblue
Copy link
Contributor Author

rdblue commented May 29, 2020

Merging to fix master. I'll clarify the comment in a follow-up.

configurations {
testCompile.extendsFrom compileOnly

compileClasspath {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is better than enforcing the exclusion via Checkstyle as this will now be evident at compile time rather than when building the project. Nice.

cmathiesen pushed a commit to ExpediaGroup/iceberg that referenced this pull request Aug 19, 2020
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