-
Notifications
You must be signed in to change notification settings - Fork 29
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
GLSP-1380: Remove dependency to Guava #244
Conversation
Removes the dependency to Google Guava. We mostly used collection creation which can be done with native Java now almost as conveniently. Other than that we only used the BiMap for source model indexing in EMF. While Guavas BIMaps are powerful most of their functionality is not required for our usecases therefore the useage got replaced with a custom BiIndex implementation. - Replaces usage of Lists.newArrayList etc. with Java native calls - Guava Preconditions were only used in two places -> replaced with in method exception handling - Introduce a `BIIndex` helper class that serves as a replacement for Guavas BIMap in source model indexing - Add test cases for the new BiIndex - Update target platforms Fixes eclipse-glsp/glsp#1380 Contributed on behalf of AxonIvy AG
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.
Code looks good to me, in general. Everything still seems to work as expected. I just a minor nitpick and I'm wondering about the dependency-reduced-pom.xml
. Could you please elaborate?
examples/org.eclipse.glsp.example.workflow/dependency-reduced-pom.xml
Outdated
Show resolved
Hide resolved
plugins/org.eclipse.glsp.layout/src/org/eclipse/glsp/layout/ElkLayoutEngine.java
Outdated
Show resolved
Hide resolved
tests/org.eclipse.glsp.server.test/src/org/eclipse/glsp/server/utils/BiIndexTest.java
Show resolved
Hide resolved
This should probably mentioned in the changelog or do you think it won't be necessary? |
Yes, you are right. I forgot to tick the checkboxes. Should definitely be added to the changelog |
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.
LGTM!
What it does
Removes the dependency to Google Guava. We mostly used collection creation which can be done with native Java now almost as conveniently. Other than that we only used the BiMap for source model indexing in EMF. While Guavas BIMaps are powerful most of their functionality is not required for our usecases therefore the useage got replaced with a custom BiIndex implementation.
BiIndex
helper class that serves as a replacement for Guavas BiMap in source model indexingFixes eclipse-glsp/glsp#1380
Contributed on behalf of AxonIvy AG
How to test
Follow-ups
Changelog