-
-
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
Remove netty transport, rework shading #1307
Conversation
# Conflicts: # core/build.gradle
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.
Provides such a nice cleanup and removes complexity from the build, thanks!
project.afterEvaluate { | ||
dependencies { | ||
for (id in project.configurations.compile.resolvedConfiguration.resolvedArtifacts*.moduleVersion*.id) { | ||
exclude(dependency("${id.group}:${id.name}")) |
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 is interesting - what's this doing? It looks like we're excluding all transitive deps. Is this in order to make all deps explicit?
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.
Read it as: "shade everything, except the dependencies from the compile
scope". It means that if docker-java adds a dependency on X, it will never appear in our dependencies unless we add it to compile
scope (aka add it as a public dependency of the library)
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.
Got it 👍
Would you like to do the honours and merge, @bsideup? 😄 |
@rnorth :D yeah, waiting for CI status after the master merge, will merge shortly after |
Releasing this in 1.11.0 🎉 |
We were running OkHttp transport by default since version 1.9.0 without any (reported) issues.
Since Netty shading caused some problems in the past (especially with native dependencies), it doesn't make sense to keep it as an alternative transport because there aren't any features that supported by Netty but not by OkHttp.
Note that once docker-java is split into modules, we should contribute the OkHttp transport back to docker-java and remove the shading all together.