-
Notifications
You must be signed in to change notification settings - Fork 191
Removed Guava (left in tests), bump Gradle and all deps. (this is 2nd part of https://github.com/java-json-tools/jackson-coreutils/pull/3) #47
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
Conversation
2. Bump gradlew to 5.0 and adjust scripts for compatibility 3. Builds and tests run now on Java 6-11 4. Bump all versions to top available.
CI failed due to version 1.10-(SNAPSHOT) of |
…oreutils:1.10-SNAPSHOT from my earlier pull #3
@huggsboson @fge hey guys, please, I see a lot of things simply hang on |
It would be really great if we can get Guava out from this library. https://nvd.nist.gov/vuln/detail/CVE-2018-10237 |
@mmannerm I have no idea why these repos are so dead. completely dead. |
I've been fiddling today with json-patch to bring it into modernity, so if you are interested in pursuing this, we can do that. I'm going to release it at least once with modern Guava to avoid the CVEs. |
@Capstan I have asked before, but didn't fully get the answer. The whole idea behind was to remove Guava completely from (as many as possible) projects as it is totally non-needed. I felt we are on the same page here simply because why not to remove the biggest unnecessary dependency when you easily can?? I see in many repos/discussions in recent time this slowly boiled down to [at least] updating Guava. Boiling this down to updating Guava neglect the point of work and discussion. The benefits are clear and I see not reason to go conservative here. Can we get back and stick to "remove Guava" as the goal? Or maybe I interpreted incorrectly those comment/PRs by you? |
@soberich I answered previously at java-json-tools/jackson-coreutils#3 (comment): Where there are CVEs, the easiest route forward is to update the dependencies to avoid the CVEs, even if it maintains a dependency on Guava, especially since removing Guava is likely to break forward compatibility. Updating to a modern Guava to allow people to sync and be safe does not prohibit removing it in a follow-on commit. |
|
||
import javax.annotation.Nullable; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.*; |
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.
I've exploded these into individual imports for clarity.
Rolled the above into #66 |
Hi,
We really want to remove Guava as it is on of the largest deps in war though required by this dependency (and jackson-coreutils). This would ease the choice to depend or not on Guava.
Please kindly review.
N.B. NOW BOTH
jackson-coreutils
andjson-patch
could be left without Guava.See java-json-tools/jackson-coreutils#3
No "optimizations" made. Just a plain migration with practically coping and pasting logic from Guava's methods (Guava is a great library but we just have a JAX-RS web-app and Guava is a too big contributor to classpath).
Overview.1. Removed Guava (left in tests)
2. Bump gradlew to 5.0 and adjust scripts for compatibility
3. Builds and tests run now on Java 6-11
4. Bump all versions to top available.
EDIT: @huggsboson @fge just to ping you guys.