eclipse-mylyn / org.eclipse.mylyn

Eclipse Public License 2.0
14 stars 9 forks source link

Remove guava in wikitext #562

Closed stephan-herrmann closed 3 months ago

stephan-herrmann commented 3 months ago

https://github.com/eclipse-mylyn/org.eclipse.mylyn/issues/521

stephan-herrmann commented 3 months ago

Various strategies applied:

stephan-herrmann commented 3 months ago

My goal is to find out if https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1472 could be implemented with wikitext (markdown) and without guava. For that task I don't expect to need any of the *.ui bundles, as rendering to HTML should be fully sufficient for JDT. In particular no markdown editing is planned.

merks commented 3 months ago

This looks promising.

gnl42 commented 3 months ago

How will this work live with #521?

merks commented 3 months ago

How will this work live with #521?

If I understand correctly, it provides direct implementations that address https://github.com/eclipse-mylyn/org.eclipse.mylyn/issues/521#issuecomment-2109504799 for the non-ui parts.

stephan-herrmann commented 3 months ago

I admit I found #530 only after submitting this PR :)

So the current PR is much narrower in scope, but succeeds to fully eliminate guava dependency from the core plugin without adding any other dependency.

I'm open to discussions to find the best of all worlds. I'm driven by the fact that JDT needs markdown rendering by September the latest.

ruspl-afed commented 3 months ago

@stephan-herrmann thank you for your PR! Perhaps it was my fault to not mention an existing #530 explicitly during our discussion

I would prefer to have #530 merged first, since it was prepared earlier in the scope of #521 and using the same approach as we already have in other parts of Mylyn codebase, and then discuss your change on top of it to reduce the number of required 3rd party libraries. I hope this is fine with you.

How will this work live with #521?

@gnl42 could you please resolve conflicts for #530 so we can merge it and proceed with further discussion?

stephan-herrmann commented 3 months ago

I would prefer to have #530 merged first, since it was prepared earlier in the scope of #521 and using the same approach as we already have in other parts of Mylyn codebase, and then discuss your change on top of it to reduce the number of required 3rd party libraries. I hope this is fine with you.

Sure, I certainly didn't mean to jump the queue :)

How will this work live with #521?

@gnl42 could you please resolve conflicts for #530 so we can merge it and proceed with further discussion?

If in doubt, some of the more involved changes, that might require a discussion, could perhaps be postponed for later?

gnl42 commented 3 months ago

530 has been replaced by #565

gnl42 commented 3 months ago

@stephan-herrmann Can you recreate your changes against the current main?

stephan-herrmann commented 3 months ago

@stephan-herrmann Can you recreate your changes against the current main?

See new PR #568

stephan-herrmann commented 3 months ago

Hopefully #568 will take this to completion.