-
Notifications
You must be signed in to change notification settings - Fork 55
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 usage of Guave #865
Comments
Is it actually used? https://github.com/search?q=repo%3Aeclipse%2Flsp4e+guava&type=code does not show any usage in Java files. |
Guava is not in the packagename. You should search for com.google.common . |
Still only 8 usages. Maybe @ptziegler can help here also after his experience with wb? |
From what I can tell, only the following part of Guava is used is lsp4e:
If necessary, the first four could probably be rewritten to not use Guava anymore, but except for Functions.identity() (which should be identical to java.util.function.Function.identity()), I don't think there is a drop-in replacement. The usage of ranges pretty much requires Guava (or a library with similar functionality). Whether a migration is worth not carrying a 2MB dependency around, I can't answer.
Both reason don't seem to apply here. |
We are using guava in tm4e (CacheBuilder, GsonBuilder, Iterators, Strings, Lists) which is usually used together with lsp4e. I don't see us removing it from tm4e without introducing other new dependencies. So I don't really see the benefit of removing it from lsp4e. |
See by the way m2e uses guava as well for the sake of |
Also see for a discussion where the usage of guava currently prevents usage of LSP4e in eclipse-platform while it seems a technology that should be used even more there and currently requires several plugins to ship their own what can then create even more problems of competing versions. |
That's right but I think this can be reworked to use a LinkedHashMap instead. And if only a more complex cache is necessary than can reasonably be implemented based on standard java, the caffeine library might be worth a consideration. It is similar to guava's caches but much smaller. |
The problem with maps is that these are not reentrant safe and that was the problem with |
But it is not a requirement to use tm4e so I still thing it would be beneficial especially as tm4e now has removed guava dependency: |
That was a fast turnaround. Thanks @sebthom for the removal of Guava in tm4e. |
@vogella I thought that Gson is part of guava, but that is not the case. so removing guava was not as complicated as anticipated. |
IIRC I saw recently a discussion from @akurtakov that the usage of Guave in lsp4e would be desired. Opening this issue to capture this.
@ptziegler does the same at the moment for WindowBuilder see for example eclipse-windowbuilder/windowbuilder#615
The text was updated successfully, but these errors were encountered: