Bug 569421 - Move com.ibm.icu from org.eclipse.e4.rcp feature to org.eclipse.rcp feature
Summary: Move com.ibm.icu from org.eclipse.e4.rcp feature to org.eclipse.rcp feature
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: IDE (show other bugs)
Version: 4.17   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: 4.19 M3   Edit
Assignee: Lars Vogel CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks:
 
Reported: 2020-12-03 05:18 EST by Lars Vogel CLA
Modified: 2021-01-11 07:42 EST (History)
4 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2020-12-03 05:18:49 EST 
RCP application do not require com.ibm.icu anymore since databinding made it optional. We should remove it from the org.eclipse.e4.rcp.

As JDT still needs it (I think), we must contribute it via another feature for the IDE. 

Suggestions?
Comment 1 Eclipse Genie CLA 2020-12-03 05:24:03 EST 
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/173295
Comment 2 Matthias Becker CLA 2020-12-03 07:09:14 EST 
(In reply to Lars Vogel from comment #0)
> RCP application do not require com.ibm.icu anymore since databinding made it
> optional. We should remove it from the org.eclipse.e4.rcp.
> 
> As JDT still needs it (I think), we must contribute it via another feature
> for the IDE. 
> 
> Suggestions?

Wouldn't it be correct that JDT adds it to its own feature if they need it?
Comment 3 Lars Vogel CLA 2020-12-03 07:47:06 EST 
(In reply to Matthias Becker from comment #2)
> Wouldn't it be correct that JDT adds it to its own feature if they need it?

I think it should rather be provided by a platform feature, other plug-ins like CDT might depend on this plug-in provides by the platform.
Comment 4 Lars Vogel CLA 2021-01-08 05:22:07 EST 
(In reply to Lars Vogel from comment #0)
> RCP application do not require com.ibm.icu anymore since databinding made it
> optional. We should remove it from the org.eclipse.e4.rcp.
> 
> As JDT still needs it (I think), we must contribute it via another feature
> for the IDE. 
> 
> Suggestions?

Alex, can you suggest a better feature to contain com.ibm.icu? This would save > 10 MB from ~17 for a minimal e4 RCP application. Maybe we can put it into the e3 feature org.eclipse.rcp?
Comment 5 Alexander Kurtakov CLA 2021-01-08 05:24:20 EST 
(In reply to Lars Vogel from comment #4)
> (In reply to Lars Vogel from comment #0)
> > RCP application do not require com.ibm.icu anymore since databinding made it
> > optional. We should remove it from the org.eclipse.e4.rcp.
> > 
> > As JDT still needs it (I think), we must contribute it via another feature
> > for the IDE. 
> > 
> > Suggestions?
> 
> Alex, can you suggest a better feature to contain com.ibm.icu? This would
> save > 10 MB from ~17 for a minimal e4 RCP application. Maybe we can put it
> into the e3 feature org.eclipse.rcp?

If no plugins in e4.rcp require icu it makes perfect sense to move it o.e.rcp.
Comment 6 Alexander Kurtakov CLA 2021-01-08 05:25:42 EST 
Actually is there anything in rcp that requires it strongly? I'm thinking of even moving the requirement to jdt feature but it's too much of a change now so let's go for o.e.rcp.
Comment 7 Lars Vogel CLA 2021-01-08 05:27:44 EST 
(In reply to Alexander Kurtakov from comment #6)
> Actually is there anything in rcp that requires it strongly? I'm thinking of
> even moving the requirement to jdt feature but it's too much of a change now
> so let's go for o.e.rcp.

Tbd. I don't know, I have not used e3 RCP since multiple years. I think it is fine to deliver it with this feature as long a platform require it. IIRC we still use it somewhere in platform.text in addition to JDT.
Comment 8 Matthias Becker CLA 2021-01-08 05:40:27 EST 
(In reply to Lars Vogel from comment #7)
> IIRC we still use it somewhere in platform.text in addition to JDT.

That is in DefaultTextDoubleClickStrategy that uses com.ibm.icu.text.BreakIterator
Comment 9 Eclipse Genie CLA 2021-01-08 05:43:26 EST 
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.releng/+/174443
Comment 12 Lars Vogel CLA 2021-01-08 05:52:12 EST 
(In reply to Alexander Kurtakov from comment #6)
> Actually is there anything in rcp that requires it strongly? I'm thinking of
> even moving the requirement to jdt feature but it's too much of a change now
> so let's go for o.e.rcp.

Done. Will add to N&N after a few days to ensure this does not break anything.
Comment 13 Lars Vogel CLA 2021-01-08 06:08:52 EST 
(In reply to Matthias Becker from comment #8)
> (In reply to Lars Vogel from comment #7)
> > IIRC we still use it somewhere in platform.text in addition to JDT.
> 
> That is in DefaultTextDoubleClickStrategy that uses
> com.ibm.icu.text.BreakIterator

Can you open a bug for (maybe also provide a Gerrit) that and cc Alex and myself?
Comment 14 Matthias Becker CLA 2021-01-08 07:42:46 EST 
(In reply to Lars Vogel from comment #13)
> Can you open a bug for (maybe also provide a Gerrit) that and cc Alex and
> myself?

Wasn't this discussed in bug 563123. The "solution" then was to not change the code (revert the first attempt to replace).
Comment 15 Eclipse Genie CLA 2021-01-11 07:40:03 EST 
New Gerrit change created: https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/174626