Bug 550742 - Replace EGit Early Startup action with OSGi service
Summary: Replace EGit Early Startup action with OSGi service
Status: RESOLVED FIXED
Alias: None
Product: EGit
Classification: Technology
Component: UI (show other bugs)
Version: unspecified   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 5.6   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-04 08:02 EDT by Lars Vogel CLA
Modified: 2019-09-09 03:34 EDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2019-09-04 08:02:17 EDT
In the Dart project we recently replaced a early startup with an OSGI service, see https://github.com/eclipse/dartboard/blob/master/org.eclipse.dartboard/src/org/eclipse/dartboard/ListenerService.java or https://github.com/eclipse/dartboard/commit/39b6e6f7c2ff7a2ac2ab7f27679c199894c02531 for the diff.

The user can currently deactivate the early startups, using the service would ensure the DnD functionality is always available.
Comment 1 Thomas Wolf CLA 2019-09-05 02:49:25 EDT
Is that the only benefit? Is there a problem with the IStartup implementation?

Compare https://www.eclipsezone.com//articles/extensions-vs-services/ .

We could replace it by a declarative service, but is there a compelling need to do so?
Comment 2 Michael Keppler CLA 2019-09-05 03:31:26 EDT
I thought that early startup is deprecated since a while, but I must have dreamed that, since I don't find any documentation saying so...

The summary of your linked article says: Use extension points because otherwise the eclipse IDE will not listen to your services. While that is reasonable (and I therefore also prefer extension points generally), this argument doesn't matter for replacing the IStartup extension. It's a one-shot mechanism triggered by the E4 event with no need of communicating further with the host application.

I've used the same refactoring approach right now at work in a big IDE based application and I like it.
Comment 3 Lars Vogel CLA 2019-09-05 03:58:23 EDT
(In reply to Michael Keppler from comment #2) 
> The summary of your linked article says: Use extension points because
> otherwise the eclipse IDE will not listen to your services. While that is
> reasonable (and I therefore also prefer extension points generally), this
> argument doesn't matter for replacing the IStartup extension. It's a
> one-shot mechanism triggered by the E4 event with no need of communicating
> further with the host application.

It is also possible to communicate with your services via a IEclipseContext.get(YourService.class) call. For example, you can use Workbench.getService to access your OSGi services.
Comment 4 Thomas Wolf CLA 2019-09-05 05:46:26 EDT
How about a headless application? I presume the UIEvents.UILifeCycle.APP_STARTUP_COMPLETE just would never be fired, thus also side-stepping the problem that was mentioned in bug 542476?
Comment 5 Lars Vogel CLA 2019-09-05 05:52:45 EDT
(In reply to Thomas Wolf from comment #4)
> How about a headless application? I presume the
> UIEvents.UILifeCycle.APP_STARTUP_COMPLETE just would never be fired, thus
> also side-stepping the problem that was mentioned in bug 542476?

I had a brief look yesterday into the early startup extension and I think it only registers a DnD listener which IMHO should anyhow not registered in an headless environment. So the not being called in headless node would be a plus.
Comment 6 Thomas Wolf CLA 2019-09-05 05:54:27 EDT
Yes, it should not be called. In fact, as I pointed out in bug 542476, calling an IStartup in a headless application is a bug in EASE.
Comment 7 Lars Vogel CLA 2019-09-05 06:09:22 EDT
(In reply to Thomas Wolf from comment #6)
> Yes, it should not be called. In fact, as I pointed out in bug 542476,
> calling an IStartup in a headless application is a bug in EASE.

I think it depends which IApplication is used. AFAICS our platform code only sends it out in PartRenderingEngine, which "should" only be used if you renderer something.
Comment 8 Eclipse Genie CLA 2019-09-08 15:31:57 EDT
Gerrit change https://git.eclipse.org/r/148958 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=b3116696697b28bef9710da28ed4af9441414f8f