Conversation
|
Is there a compelling reason to target 3.0.7 instead of 3.2.0? I would personally be more comfortable with 3.2.0. |
| } | ||
| } | ||
|
|
||
| private static class Version18OrLower extends GuavaCompatibility { |
There was a problem hiding this comment.
Nit: adding @SuppressWarnings("deprecation") here would spare us a couple warnings.
| private static Manifest loadGuavaManifest() { | ||
| InputStream in = null; | ||
| try { | ||
| Enumeration<URL> resources = Resources.class.getClassLoader() |
There was a problem hiding this comment.
Resources is itself part of Guava, and is marked @Beta. Why not use GuavaCompatibility.class.getClassLoader()?
There was a problem hiding this comment.
I hadn't noticed it was beta. I think it's better to use a Guava class to make sure it's the classloader that loaded Guava, I've changed it to Preconditions.
|
|
||
| @Override | ||
| public <I, O> ListenableFuture<O> transformAsync(ListenableFuture<I> input, AsyncFunction<? super I, ? extends O> function, Executor executor) { | ||
| return Futures.transform(input, function, executor); |
There was a problem hiding this comment.
Found one occurrence of this method in com.datastax.driver.core.AsyncQueryTest#connectAndQuery
| * They are available in some versions of Java/Guava, but not across all versions ranges supported by the driver, hence | ||
| * the custom implementation. | ||
| */ | ||
| public class MoreObjects { |
There was a problem hiding this comment.
Since it's public, maybe we could add javadocs for methods in this class, even if I concede that their purposes are pretty obvious.
| @@ -40,8 +40,8 @@ | |||
| public class LimitingLoadBalancingPolicy extends DelegatingLoadBalancingPolicy { | |||
There was a problem hiding this comment.
The import import com.google.common.collect.Sets is unused.
|
In addition to my review, we should also
|
|
Addressed all feedback. |
That's a good point, it doesn't break API compatibility, but is more of a new capability than a hotfix so 3.2.0 does seem more appropriate. |
|
CI looks happy with guava 16.0.1, 17.0, 18.0, 19.0, 20.0 and 21.0, excellent work! |
|
Glad to see this approved! Is there any chance for a release in the near future? |
|
@Xaerxess no timeframe yet, but it shouldn't be long. |
|
Squashed and rebased on top of 3.x instead of 3.0.x. |
|
Note to reviewers: I added an extra commit to fix OSGi tests and also to revert the Import-Package instruction that we use to create our bundles. We need to explicitly declare that the driver bundle accepts any Guava version in the range [16.0.1,21) so I modified the instructions to read that. |
Based on original work by @mspangdal.
Based on original work by @mspangdal.