search

eclipse-mylyn / org.eclipse.mylyn

Eclipse Public License 2.0
13 stars 9 forks source link

Remove guava in wikitext, part 1 #521

Open gnl42 opened 2 months ago

gnl42 commented 2 months ago

Need to replace guava uses in wikitext with native/commons alternatives

gnl42 commented 1 month ago

So I've run into a bit of snag. There are a couple of guava classes used by wikitext that don't have direct (or even simple ) replacement code. Specifically CharMatcher/Escapers

I'm not sure what's the best approach at this point:

  1. Leave Guava jar in place (wikitext code is the only user)?
  2. Take the time and effort to come up with custom alternatives?
  3. Extract the appropriate classes from Guava and add them to wikitext (with appropriate mods)?

I have a working Poc of the third option, 25 classes in total. It's an ugly hack, but it works. Potentially would allow for a refactor/rewrite in the future.

Thoughts? @BeckerFrank @akurtakov @merks @wimjongman

akurtakov commented 1 month ago

I would vote for 1. as adding 25 classes of thirdparty code will exclude us from getting bugfixes that might/will happen upstream. 2. is actually smth that has been happening in the last years although a bit too slow.

merks commented 1 month ago

3 sounds nasty 🤮 2 sounds like a lot of work and might end up with a similar but unproven amount of new code. 1 would leave us with the problem we hoped to eliminate.

I leave decisions to the folks doing the hard development work.

wimjongman commented 1 month ago

3 is an implementation of 2. Why do we want to ditch Guava again?

akurtakov commented 1 month ago

3 is an implementation of 2.

It could be or not. I've been replacing Guava usages with new Java versions API and keeping Guava for the rest (e.g https://github.com/eclipse-mylyn/org.eclipse.mylyn/commit/4fd84e30acddcf4c224a4a63b4487119f343aa14 https://github.com/eclipse-mylyn/org.eclipse.mylyn/commit/9ad02738c52b58fbcad91743f3db0f1355c7dc18 https://github.com/eclipse-mylyn/org.eclipse.mylyn/commit/2f0cc660aa4f8d50a11e37de7fc151b59bbc651d https://github.com/eclipse-mylyn/org.eclipse.mylyn/commit/803292b246c65c115333d12b2730a3b9759bb397 and others), in that case - it's a slow reduction of guava usage without coping code to be maintained by the project.

wimjongman commented 1 month ago

in that case - it's a slow reduction of guava

Sure, that is reasonable. George, Are you ok with leaving Guava in for the other unsupported cases?

gnl42 commented 1 month ago

@wimjongman I've left the hard cases alone.