-
Notifications
You must be signed in to change notification settings - Fork 56
More v2 changes #55
More v2 changes #55
Conversation
|
|
||
| task wrapper(type: Wrapper) { | ||
| gradleVersion = '2.0' | ||
| gradleVersion = '2.14-rc-3' |
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 the version mentioned here: https://github.com/johnrengelman/shadow
| classifier = '' | ||
|
|
||
| // Shade all jars except for launchdarkly | ||
| relocate('com', 'com.launchdarkly.shaded.com') { |
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.
The potential problem with shading Gson is that we return a Gson type (JsonElement) from our public interface: https://github.com/launchdarkly/java-client/blob/fa61ceb5f343c53db01f88b417a623575ab9fc58/src/main/java/com/launchdarkly/client/LDClient.java#L298-L298
This means that if a user's code already imports Gson this JsonElement is a different type than they might be expecting.
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.
Hmm. I hadn't considered that when I made the suggestion to shade Gson. I think we should not shade it then-- leaking a shaded class into an external interface isn't a good practice.
In addition to just having the wrong type, interacting with the JsonElement is going to be hard-- you'll have to manipulate the shaded API, which exposes more shaded footprint.
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.
It might be appropriate to reconsider this approach. In my project, SBT (sbt-assembly) got confused by launchdarkly-client depending on com.google.code.gson:gson, but both artifacts included com/google/gson/Gson.class etc.
| relocate('okhttp3', 'com.launchdarkly.shaded.okhttp3') | ||
| relocate('okio', 'com.launchdarkly.shaded.okio') | ||
| relocate('org', 'com.launchdarkly.shaded.org') { | ||
| exclude("org.slf4j.*") |
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.
Good catch on excluding the logging facade.
|
👍 |
add more flag evaluation unit tests
More v2 changes. See comments in code