Replace Guava dep with local RateLimiter implementation copy-paste#351
Merged
domesticmouse merged 1 commit intogooglemaps:masterfrom Oct 2, 2017
apjanke:local-rate-limiter-impl
Merged
Replace Guava dep with local RateLimiter implementation copy-paste#351domesticmouse merged 1 commit intogooglemaps:masterfrom apjanke:local-rate-limiter-impl
domesticmouse merged 1 commit intogooglemaps:masterfrom
apjanke:local-rate-limiter-impl
Conversation
…ndency This replaces the Guava dependency with a local copy of a minimal set of Guava classes/methods needed to implement RateLimiter, all under the new com.google.maps.internal.ratelimiter package. Code was copied from Guava 23.0 and hand-modified as necessary to get it running under Java 7 and without Guava-supplied annotations.
Closed
domesticmouse
approved these changes
Oct 2, 2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copies the RateLimiter implementation from Google Guava 23.0 to local classes and drops the Guava dependency. This removes the need for the Guava dependency, which some users feel is heavyweight, and may cause Java 7 compatibility issues. Addresses #285, especially #285 (comment).
This PR replaces the Guava dependency with a local copy of a minimal set of Guava classes/methods needed to implement RateLimiter, all under the new com.google.maps.internal.ratelimiter package. Code was copied from Guava 23.0 and hand-modified as necessary to get it running under Java 7 and without Guava-supplied annotations.
Doing a mechanical repackaging of RateLimiter and its dependencies would have been quite large (187 classes in the transitive dependency), so I started with
RateLimiterandSmoothRateLimiterand pared their dependencies down by hand on a per-method basis, sticking all the dependencies in thecom.google.maps.internal.ratelimiterpackage instead of breaking them out in separate packages like they are in the original Guava library.Only four full classes (
RateLimiter,SmoothRateLimiter,StopwatchandTicker) and a handful of methods fromLongMath,Platform, andPreconditionshad to be pulled over. This seems like a reasonable amount of code to be adding to thegoogle-maps-services-java` codebase for manual maintenance. And there weren't a whole lot of edits besides removing annotations, so doing diff inspections against future releases of Guava should be manageable. I think this might be an acceptable maintenance burden in exchange for removing the Guava dependency.I wasn't sure of exactly how to handle the copyright notices in the new files adapted from the Guava library. Since this is a derived work, and the bulk of the code is from the original library, I think the thing to do is to retain the original copyright notice, and to add a new 2017 copyright notice to reflect the modifications in the derived work. I've done so with the files here; let me know if it should be different.
All tests pass for me locally, running against Oracle JDK 1.7.0_79 on macOS.