Skip to content
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 guava in wikitext, part 1 #521

Open
gnl42 opened this issue May 10, 2024 · 7 comments · May be fixed by #568
Open

Remove guava in wikitext, part 1 #521

gnl42 opened this issue May 10, 2024 · 7 comments · May be fixed by #568
Assignees
Labels
enhancement New feature or request

Comments

@gnl42
Copy link
Contributor

gnl42 commented May 10, 2024

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

@gnl42 gnl42 self-assigned this May 10, 2024
gnl42 added a commit that referenced this issue May 13, 2024
gnl42 added a commit that referenced this issue May 13, 2024
gnl42 added a commit that referenced this issue May 13, 2024
@gnl42
Copy link
Contributor Author

gnl42 commented May 14, 2024

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
Copy link
Contributor

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
Copy link
Contributor

merks commented May 14, 2024

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
Copy link
Member

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

@akurtakov
Copy link
Contributor

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 4fd84e3 9ad0273 2f0cc66 803292b and others), in that case - it's a slow reduction of guava usage without coping code to be maintained by the project.

@wimjongman
Copy link
Member

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 added a commit that referenced this issue May 20, 2024
Task-Url: #521
@gnl42 gnl42 linked a pull request May 20, 2024 that will close this issue
@gnl42
Copy link
Contributor Author

gnl42 commented May 20, 2024

@wimjongman I've left the hard cases alone.

stephan-herrmann added a commit to stephan-herrmann/org.eclipse.mylyn that referenced this issue Jun 24, 2024
+ org.eclipse.mylyn.wikitext

eclipse-mylyn#521
@gnl42 gnl42 changed the title Remove guava in wikitext Remove guava in wikitext, part 1 Jun 28, 2024
@gnl42 gnl42 added the enhancement New feature or request label Jun 28, 2024
stephan-herrmann added a commit to stephan-herrmann/org.eclipse.mylyn that referenced this issue Jul 4, 2024
Also avoid new apache dependencies

Fully covers:
+ org.eclipse.mylyn.wikitext
+ org.eclipse.mylyn.wikitext.commonmark
+ org.eclipse.mylyn.wikitext.commonmark.tests

Tests:
+ modify MLPT.getMarkupLanguagesDuplicateNames():
  - need different language classes now
+ skip CommonMarkSpecTest line 6700 for now
+ revert expectation in LineSequenceTest.assertLookAheadFailsFast()

eclipse-mylyn#521
@stephan-herrmann stephan-herrmann linked a pull request Jul 4, 2024 that will close this issue
ruspl-afed pushed a commit to stephan-herrmann/org.eclipse.mylyn that referenced this issue Jul 6, 2024
Also avoid new apache dependencies

Fully covers:
+ org.eclipse.mylyn.wikitext
+ org.eclipse.mylyn.wikitext.commonmark
+ org.eclipse.mylyn.wikitext.commonmark.tests

Tests:
+ modify MLPT.getMarkupLanguagesDuplicateNames():
  - need different language classes now
+ skip CommonMarkSpecTest line 6700 for now
+ revert expectation in LineSequenceTest.assertLookAheadFailsFast()

eclipse-mylyn#521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants