Fixes #494 - removed unwanted classes from jaeger-thrift shadowed jar#498
Fixes #494 - removed unwanted classes from jaeger-thrift shadowed jar#498jpkrohling merged 2 commits intojaegertracing:masterfrom mdvorak:issue494
Conversation
Moved relocated packages to io.jaegertracing.vendor Excluded lombok and animal-sniffer from shadow plugin in jaeger-thrift Added slf4j as runtime non-relocated dependency Removed unused dependencies from jaeger-crossdock (change suggested by @jpkrohling) Signed-off-by: Michal Dvorak <mikee@mdvorak.org>
Codecov Report
@@ Coverage Diff @@
## master #498 +/- ##
============================================
- Coverage 88.32% 88.21% -0.11%
Complexity 498 498
============================================
Files 65 65
Lines 1859 1859
Branches 242 242
============================================
- Hits 1642 1640 -2
- Misses 139 141 +2
Partials 78 78
Continue to review full report at Codecov.
|
jpkrohling
left a comment
jpkrohling
left a comment
There was a problem hiding this comment.
On a first look, looks good to me, but I'll run this with a tester project for a sanity check.
jaeger-thrift/build.gradle
Outdated
| dependencies { | ||
| compile project(':jaeger-core') | ||
|
|
||
| compile group: 'org.slf4j', name: 'slf4j-api' |
There was a problem hiding this comment.
Shouldn't it be compileOnly? Try to run a mvn dependency:tree (or ./gradlew dependencies, if that's what your consumer application uses) and check if this version of jaeger-thrift (or jaeger-client) brings this dependency.
| dependencies { | ||
| compile project(':jaeger-client') | ||
|
|
||
| compile group: 'org.apache.thrift', name: 'libthrift', version: apacheThriftVersion |
There was a problem hiding this comment.
+1, I think I had to add this in some iteration of the previous work, to get the tests to compile/run. Apparently, it's not needed now :)
There was a problem hiding this comment.
It was your request to remove it :)
There was a problem hiding this comment.
Er... oops :)
Signed-off-by: Michal Dvorak <mikee@mdvorak.org>
|
I've forgotten slf4j version in the dependency, added.
|
|
Thanks! |
2 participants
Moved relocated packages to
io.jaegertracing.vendorExcluded lombok and animal-sniffer from shadow plugin in
jaeger-thriftAdded slf4j as runtime non-relocated dependency
Removed unused dependencies from
jaeger-crossdockI really believe
slf4j-apishould be normal runtime dependency, having it shadowed is not a good idea - it depends on external loggers to work. Relocating slf4j-api breaks that at best, crashes at worst.(note:
slf4j-apiis transitive dependency oflibthrift)