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

DRILL-8420: Remove Guava shading and patching #2786

Closed
wants to merge 3 commits into from

Conversation

jnturton
Copy link
Contributor

@jnturton jnturton commented Apr 3, 2023

DRILL-8420: Remove Guava shading and patching.

Description

Based on #2782 to enable a clean CI run.

  • Remove shaded Guava.
  • Drop Guava patches.
  • Upgrade guava to 31.1-jre.

Documentation

N/A

Testing

Existing unit tests.

@jnturton jnturton changed the title DRILL-8420: Remove Guava shading and patching, and the conjars repo DRILL-8420: Remove Guava shading and patching Apr 3, 2023
@jnturton jnturton force-pushed the 8420-guava-conjars branch 2 times, most recently from fa83a88 to 84ee8b8 Compare April 4, 2023 09:38
@cgivre
Copy link
Contributor

cgivre commented Apr 5, 2023

@jnturton Thanks for this. Aside from imports, which files were actually modified and I'll do a review?

@cgivre
Copy link
Contributor

cgivre commented Apr 5, 2023

Do we want to add this to back port to stable?

@jnturton
Copy link
Contributor Author

jnturton commented Apr 6, 2023

@cgivre

Thanks for this. Aside from imports, which files were actually modified and I'll do a review?

Yes, I'll split the import statement changes into a separate commit.

Do we want to add this to back port to stable?

I don't think so. It's not a fix and it definitely carries a risk of breakage with it. Currently the Hadoop 2 build is broken because Drill's Guava patches (but not shading) are still needed in that case so I'll set the PR to draft for the moment.

@jnturton jnturton marked this pull request as draft April 6, 2023 07:32
@cgivre
Copy link
Contributor

cgivre commented Aug 24, 2023

@jnturton Is this PR still relevant?

@jnturton
Copy link
Contributor Author

@jnturton Is this PR still relevant?

Yes, I'm going to resurrect it after all the other clean up and upgrade PRs are in.

@jnturton jnturton closed this Aug 25, 2023
@jnturton jnturton deleted the 8420-guava-conjars branch August 25, 2023 09:20
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.

2 participants