Community
Participate
Working Groups
Similar to Bug 549928 we should offer the AbstractUIPlugin#getDialogSettings functionality outside AbstractUIPlugin to allow a reduction of activator usage and therefore improve startup time of the Eclipse IDE.
This functionality is related to bundle lifecycle so I would suggest something like package org.eclipse.e4.ui.dialogs; interface IDialogSettingsService { IDialogSettings getWorkbenchSettings(Class<?> classFromBundle); } WDYT?
(In reply to Alexander Fedorov from comment #1) > This functionality is related to bundle lifecycle so I would suggest > something like > > package org.eclipse.e4.ui.dialogs; > > interface IDialogSettingsService { > IDialogSettings getWorkbenchSettings(Class<?> classFromBundle); > } > > WDYT? +1
Alexander, do you plan to provide a Gerrit for this change?
(In reply to Lars Vogel from comment #3) > Alexander, do you plan to provide a Gerrit for this change? Yes, I have it in plans. But I'm still not sure how should we bind it to the bundle lifecycle to preserve the existing behavior, if we want to preserve it. The contract now is to have one and only one instance of IDialogSettings for a bundle, load it on the first access and call the IDialogSettings#save on bundle stop. So, it grows to a service that listens for bundles state changes.
Do you need to listen for bundle state changes? If a bundle is stopped, the service is stopped. Question is if we can start the service when the bundle is started. That would be easy using an immediate component. But if we rely on the state of the Eclipse application it gets a bit more complicated. I solved that in the past via EventHandler that listens for applications startup events from the platform. But if the service is only bound to the bundle lifecycle and provides the IDialogSettings on access, an immediate component with service interface could be an option. Although, if the service is only needed on access and nothing needs to happen on bundle start, a simple service would be enough.
As far as I can see the bundle is only required to lookup settings from bundle. Loading resources is not bound to the life-cycle. Starting/stopping a bundle should therefore make no difference. So the question is: Should there be one IDialogSettings per bundle? Then a Bundletraker that registers a service for each one could be used. Or shoudl there be a IDialogSettings per Calling-Bundle? Then a ServiceFactory could be used.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166431
(In reply to Eclipse Genie from comment #7) > New Gerrit change created: > https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166431 I poked around in this code when I was looking to solve bug 564088 and moved the code out of BundleUIActivator and into a DialogSettingsUtil. There is some eclipse.ui dependencies but not much. After reading your comments this could be the workflow: 1. Get the IDialogSettingsService --> service 2. service.getDialogSettings(Bundle myBundle, boolean track); at this point, the service tracks the state of the passed bundle (boolean track added as a suggestion, when false the caller is responsible for calling #saveDialogSettings) 3. If the bundle stops the service calls the saveDialogSettings method.
(In reply to Eclipse Genie from comment #7) > New Gerrit change created: > https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166431 With the last Gerrit, I changed it into a service. It behaves the same as before but is not depending on org.eclipse.ui anymore. Please take a look at the proposed solution. The DialogSettingsService can be moved outside org.eclipse.ui. Usage for now is: reference = bundleContext.getServiceReference(DialogSettingsService.class); DialogSettingsService service = bundleContext.getService(reference); service.loadDialogSettings(getClass()); e4 can have the service injected
(In reply to Wim Jongman from comment #9) > (In reply to Eclipse Genie from comment #7) > > New Gerrit change created: > > https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166431 > > With the last Gerrit, I changed it into a service. It behaves the same as > before but is not depending on org.eclipse.ui anymore. Great :) > Usage for now is: > > reference = bundleContext.getServiceReference(DialogSettingsService.class); > DialogSettingsService service = bundleContext.getService(reference); > service.loadDialogSettings(getClass()); > > e4 can have the service injected You can also use ServiceCaller to do the work: ServiceCaller.callOnce(getClass(), DialogSettingsService.class, (service) -> dialogSettings = service.loadDialogSettings(getClass()));
@Wim thanks for working on this! I have several comments regarding implementation that I can try to summarize. The good part of code is not related to the UI and can be extracted to be testable by UI-less unit tests. I'm talking about the algo to load the settings, that can be expressed as: ``` IDialogSettings loaded = fromWorkspace()// .orElseGet(() -> fromProduct()// .orElseGet(() -> fromBundle()// .orElseGet(() -> createEmpty()))); ``` This algo can be isolated even more by taking IDialogSettings type as a parameter. Also "static" keyword is not friendly for the OSGi (and OOP) world and can be avoided. I would also extract an interface to JFace space to follow OSGi best practices and also I would put the implementation to a bundle that is visible for E4 RCPs.
(In reply to Alex Blewitt from comment #10) > > > > e4 can have the service injected 👍 > > You can also use ServiceCaller to do the work: > > ServiceCaller.callOnce(getClass(), DialogSettingsService.class, (service) -> > dialogSettings = service.loadDialogSettings(getClass())); TBH, I forgot to unget. But in this case, I did not think it makes any difference since this service is there for the whole session. I will switch to ServiceCaller, thanks. I also noted your other remarks, I will submit a new patch set today.
(In reply to Alexander Fedorov from comment #11) > @Wim thanks for working on this! 👍 > > I have several comments regarding implementation that I can try to summarize. > The good part of code is not related to the UI and can be extracted to be > testable by UI-less unit tests. I'm talking about the algo to load the > settings, that can be expressed as: > ``` > IDialogSettings loaded = fromWorkspace()// > .orElseGet(() -> fromProduct()// > .orElseGet(() -> fromBundle()// > .orElseGet(() -> createEmpty()))); > ``` > > This algo can be isolated even more by taking IDialogSettings type as a > parameter. This is a unit test expression? I will take a look at that when the code is ready. I might need your help for that. > > Also "static" keyword is not friendly for the OSGi (and OOP) world and can > be avoided. I will make the map an instance variable and the methods that use it instance methods. Otherwise, I don't see why static methods should not be used. Surely, they must be more efficient, @Alex? > > I would also extract an interface to JFace space to follow OSGi best > practices and also I would put the implementation to a bundle that is > visible for E4 RCPs. Yes, I will extract an interface but, but not in JFace. JFace is OSGi agnostic. WDYT about org.eclipse.e4.ui.services?
(In reply to Wim Jongman from comment #13) > (In reply to Alexander Fedorov from comment #11) > > @Wim thanks for working on this! > 👍 > > > > > I have several comments regarding implementation that I can try to summarize. > > The good part of code is not related to the UI and can be extracted to be > > testable by UI-less unit tests. I'm talking about the algo to load the > > settings, that can be expressed as: > > ``` > > IDialogSettings loaded = fromWorkspace()// > > .orElseGet(() -> fromProduct()// > > .orElseGet(() -> fromBundle()// > > .orElseGet(() -> createEmpty()))); > > ``` > > > > This algo can be isolated even more by taking IDialogSettings type as a > > parameter. > > This is a unit test expression? I will take a look at that when the code is > ready. I might need your help for that. This is just a suggested Java 8 code "skeleton". It may require more changes around to look exactly like this, but from my POV it looks quite good to express the idea of main algo to load the settings. > > > > I would also extract an interface to JFace space to follow OSGi best > > practices and also I would put the implementation to a bundle that is > > visible for E4 RCPs. > > Yes, I will extract an interface but, but not in JFace. JFace is OSGi > agnostic. Correct, but I do not expect any OSGi type needed to declare the interface for the service - normally it should reference only IDialogSettings type, so JFace looks legal for me to declare the interface. > > WDYT about org.eclipse.e4.ui.services? Looks good to provide the service implementation and other OSGi-related code
(In reply to Alexander Fedorov from comment #14) > > > ``` > > > IDialogSettings loaded = fromWorkspace()// > > > .orElseGet(() -> fromProduct()// > > This is just a suggested Java 8 code "skeleton". It may require more changes > around to look exactly like this, but from my POV it looks quite good to > express the idea of main algo to load the settings. I studied this. It looks like orElse is broken and orElseGet is trying to fix that mistake? I would not have guessed the behavior of orElse.
(In reply to Wim Jongman from comment #15) > (In reply to Alexander Fedorov from comment #14) > > > > > ``` > > > > IDialogSettings loaded = fromWorkspace()// > > > > .orElseGet(() -> fromProduct()// > > > > This is just a suggested Java 8 code "skeleton". It may require more changes > > around to look exactly like this, but from my POV it looks quite good to > > express the idea of main algo to load the settings. > > I studied this. It looks like orElse is broken and orElseGet is trying to > fix that mistake? I would not have guessed the behavior of orElse. `orElse` will always calculate its argument while `orElseGet` will invoke the given Supplier only if the Optional is empty. This is the OOP replacement for the procedural code we have in AbstractUIPlugin#loadDialogSettings, it implies that we have a number of ways to obtain the settings and that we would like to try them in the given order.
I pushed a new patch. The AbstractUIPlugin methods that handle DialogSettings are now deprecated. The method to get the dialog settings is moved to PlatformUI. PlatformUI.getDialogSettings(Bundle) The work is done in DialogSettingsService which is not an OSGI service but an internal singleton class. This class should be moved to an e4 bundle on which ui.workbench and e4.ui.workbench both depend. When the dialog settings are requested the bundle is "tracked". When the bundle stops, the dialog settings are saved automagically (like before in the stop method of AUIPlugin). For E4, the IDialogSettings should be pushed in the bundle context, if we had one, so that they can be injected. This is a separate bug.
Thanks Wim, to which bundle should the new class be moved?
(In reply to Lars Vogel from comment #18) > Thanks Wim, to which bundle should the new class be moved? Yes that is my question as well. Here are the downstream e4 candidates of org.eclipse.ui.workbench org.eclipse.e4.core.services org.eclipse.e4.core.contexts org.eclipse.e4.core.di org.eclipse.e4.ui.workbench.swt org.eclipse.e4.ui.di org.eclipse.e4.ui.model.workbench org.eclipse.e4.ui.bindings org.eclipse.e4.ui.workbench3, org.eclipse.e4.ui.workbench.addons.swt; org.eclipse.e4.ui.services org.eclipse.e4.core.di.extensions
(In reply to Wim Jongman from comment #19) > (In reply to Lars Vogel from comment #18) > > Thanks Wim, to which bundle should the new class be moved? > > Yes that is my question as well. Here are the downstream e4 candidates of > org.eclipse.ui.workbench > > org.eclipse.e4.core.services > org.eclipse.e4.core.contexts > org.eclipse.e4.core.di > org.eclipse.e4.ui.workbench.swt > org.eclipse.e4.ui.di > org.eclipse.e4.ui.model.workbench > org.eclipse.e4.ui.bindings > org.eclipse.e4.ui.workbench3, > org.eclipse.e4.ui.workbench.addons.swt; > org.eclipse.e4.ui.services > org.eclipse.e4.core.di.extensions "org.eclipse.e4.ui.services" from the list looks relatively good, perhaps "org.eclipse.e4.ui.dialogs" could be even better
I want to add IDialogSettings to the e4 context but this needs to be on a per bundle basis. I recall there is an IContextRunnable or something that I can put in the context which delivers the correct object request for @Inject. I can't find info about it.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166431 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8266c76c0792ed71ee13bff1514e3c5cab3f4883
New Gerrit change created: https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/170903
Gerrit change https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/170903 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=9c030229fda79ca3eaf061495cfeede9783fc1ef
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/170931
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/170931 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=f9dcf4b8a616d5210c6ca8fb8b7a7d9ae5ba2278
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ua/+/170972
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ua/+/170972 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ua.git/commit/?id=b6cd408e68c3beb9c01133effbaa8f95bad18628
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ua/+/170989
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ua/+/170989 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ua.git/commit/?id=fe8e4cdd5f876e5f9b0b9e3dc9008580967527de
(In reply to Wim Jongman from comment #21) > I want to add IDialogSettings to the e4 context but this needs to be on a > per bundle basis. I recall there is an IContextRunnable or something that I > can put in the context which delivers the correct object request for @Inject. > > I can't find info about it. Maybe we can have a method in IDialogSettings which takes a bundle or class and returns the correct setting?
(In reply to Lars Vogel from comment #31) > (In reply to Wim Jongman from comment #21) > > I want to add IDialogSettings to the e4 context but this needs to be on a > > per bundle basis. I recall there is an IContextRunnable or something that I > > can put in the context which delivers the correct object request for @Inject. > > > > I can't find info about it. > > Maybe we can have a method in IDialogSettings which takes a bundle or class > and returns the correct setting? IDialogSettingsProviders are instantiated per bundle. Maybe we can make a context function that provides these. Clients then also don't need to worry about where to save the settings. (It would also be nice if E4 had a bundle context.)
(In reply to Wim Jongman from comment #32) > IDialogSettingsProviders are instantiated per bundle. Maybe we can make a > context function that provides these. Clients then also don't need to worry > about where to save the settings. I don't think this can work, as the IEclipseContext hierarchy does not consider bundle context. Maybe we can define a IDialogSettingsService which allows access per bundle? IDialogSettingsService could be placed in the MApplication context.
The ContextFunction can query the context for a MContribution and then use the class supplied in getContributionURI to get hold of the bundle-context of the contribution class. If you fetch the service through this bundle-context you will get the right Bundle Service...
(In reply to Wim Jongman from comment #17) > I pushed a new patch. The AbstractUIPlugin methods that handle > DialogSettings are now deprecated. The method to get the dialog settings is > moved to PlatformUI. > > PlatformUI.getDialogSettings(Bundle) > > The work is done in DialogSettingsService which is not an OSGI service but > an internal singleton class. This class should be moved to an e4 bundle on > which ui.workbench and e4.ui.workbench both depend. > > When the dialog settings are requested the bundle is "tracked". When the > bundle stops, the dialog settings are saved automagically (like before in > the stop method of AUIPlugin). > > For E4, the IDialogSettings should be pushed in the bundle context, if we > had one, so that they can be injected. This is a separate bug. Let me express my *extreme* displeasure at this deprecation. The replacement is entirely new, so I cannot use it in any project. It's not as if for each and every release everyone downstream from you wants to change their lower bounds to the latest release. Pretty much no one does that and pretty much no one wants to do that. So now I have to modify 36 uses of this in EMF, XSD, and Oomph just to suppress the warning. I really do have better things to do and I am really very tired of dealing with deprecation warnings, especially ones that appear to me to be entirely gratuitous. I don't know what value you guys think this has because in the end the more pointless noise you generate the more everyone learns to ignore your pointless noise and eventually people will think that you only generate pointless noise and will stop paying attention entirely. We're probably already at that stage, i.e, no one cares that something is deprecated, if they even notice; mostly we have all just learned to ignore you and the growing list of things we've been doing for 20 years that are suddenly bad and should be changed to a new and improved good way. But it's even worse in this case because EMF generates code that uses this method, so now it will generate code with warnings. I don't like that. My users won't like that. This makes me angry, very angry. You are killing me softly and in my opinion are killing Eclipse with kindness. Please remove the deprecation.
I should also point out the 350 calls to this method in my platform SDK workspace. Surely we all have better things to do than to inline code "PlatformUI.getDialogSettingsProvider(getBundle()).getDialogSettings()" in hundreds of places. This will not make any code more readable and will not generate less byte code. I can't see any improvement that would come from such an activity. I think this demonstrates well the pointlessness of the noise. The fact that the noise is ignored even by the team itself suggests that it's very likely ignored by everyone. Also, when I look more closely, I see that a method like loadDialogSettings is no longer called, even though this was previously been a hook that one could override, so the changes are potentially (though very unlikely) break API behavior too.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/172631
Hey Ed, thanks for the rant :) You did not have your coffee yet? ;) > The replacement is entirely new, so I cannot use it in any project. If you see the warning then the replacement is also available. > so the changes are potentially (though very unlikely) break API behavior too. That is not good. I will take look. > I can't see any improvement that would come from such an activity. As stated in the description. The reason for the deprecation is that we want to discourage the use of Activators which slow down startup. The plan is to remove all AbstractUIPlugin activators in time. In order to get dialog settings people HAD to use AbstractUIPlugin. Moving the code also makes the dialog settings available for people that do not want to use eclipse.ui I don't see why this is such a big deal, but if it means that much to you, I'm fine with removing the deprecation. In the light of the goal, what are your recommendations to force people away from this class other than @Deprecated?
(In reply to Wim Jongman from comment #38) > Hey Ed, thanks for the rant :) You did not have your coffee yet? ;) > > > The replacement is entirely new, so I cannot use it in any project. > > If you see the warning then the replacement is also available. As I elaborated, to use the replacement one must change the lower bound to the version of the platform with the replacement. That's the current version! That's simply not acceptable for the projects that I maintain. EMF runs on Helios. Oomph runs on Juno. > > > so the changes are potentially (though very unlikely) break API behavior too. > > That is not good. I will take look. > I doubt it really affects anyone so I'd be more concerned about annoying other people than about breaking them with a change that 99.9999% of the people (likely 100%) won't notice, if you don't generate noise about it. > > I can't see any improvement that would come from such an activity. > > As stated in the description. The reason for the deprecation is that we want > to discourage the use of Activators which slow down startup. The plan is to > remove all AbstractUIPlugin activators in time. In order to get dialog > settings people HAD to use AbstractUIPlugin. > You are killing Eclipse with kindness. You should feel free to invest however much time on whatever activity you feel gets the most bang for your investment buck and I applaud your contribution toward that effort. But please do no make assumptions about anyone else's investment capital. I do not have any resource (none, zero, nada, nix) to invest in reworking EMF's generator. You should assume that the majority of your consumer base also does not have such resource; they just hope that each release of Eclipse is better and that their software will just keep working. > Moving the code also makes the dialog settings available for people that do > not want to use eclipse.ui > That all sounds fine and good, but it does not sound like good a reason why I can't call getDialogSettings anymore. > I don't see why this is such a big deal, but if it means that much to you, > I'm fine with removing the deprecation. > As you can no doubt tell, it really makes me *freak out*. As I've explained, the EMF generator generates uses of this. The generator also generates plugins. There will be many *thousands* of such things out there in the real world not to mention in all the downstream projects: most of the uses of this in Oomph come from EMF's generator and it's of course horrible to change generated code if only to suppress a deprecation warning. > In the light of the goal, what are your recommendations to force people away > from this class other than @Deprecated? Don't force people. This is the problem. If you pay me, I will gladly do things for you. But you aren't paying me nor anyone else. You are providing a service that should make me happy. But if you think to create work for other people because it's in their best interest, then you are misguided in that thinking. People will stop listening to you if you generate too much noise and they most certainly don't need you to schedule work for them. Just make Eclipse itself better---provide alternatives, remove all your own uses of activators, make Eclipse faster, fix bugs, provide new features I can actually see a user---but please, please, please don't make me or anyone else do something unless there is a *ultra compelling* need in the platform itself, i.e., we'll save so MB of dead code or save so many person weeks of "pointless" maintenance work... So yes, please, please, please remove the deprecation. Warnings in my code freak me out. I am compelled to take action because if I don't, I will never notice any thing that you really need me to notice.
(In reply to Ed Merks from comment #39) It's all said, but I feel a strong need to second all of your arguments! My CDO users often tell me that they don't upgrade to current CDO versions because they fear the required Platform upgrade. I used to reply that CDO has no strong preference for any particular Platform version. I really wouldn't want to bump my lower bounds for nothing.
(In reply to Ed Merks from comment #39) > Just make Eclipse itself better---provide alternatives, > remove all your own uses of activators, make Eclipse faster, fix bugs, > provide new features I thought I did all of this with this patch. > In the light of the goal, what are your recommendations to force people away > from this class other than @Deprecated? You focused on the word "force". Let me rephrase: what other tool do we have to let people know this class/method is not effective?
(In reply to Wim Jongman from comment #41) > (In reply to Ed Merks from comment #39) > > > Just make Eclipse itself better---provide alternatives, > > remove all your own uses of activators, make Eclipse faster, fix bugs, > > provide new features > > I thought I did all of this with this patch. > I did not intend to criticize the quality of the work, nor to question the reasonableness of the underlying goals. > > > In the light of the goal, what are your recommendations to force people away > > from this class other than @Deprecated? > > You focused on the word "force". Let me rephrase: what other tool do we have > to let people know this class/method is not effective? The only tool in the end is documentation, but people don't read it even when it's available and well written. "You can lead a horse to water but you can't make it drink." Setting the best example ourselves in our own code base is also generally effective; people copy the style and patterns that they see when they do new things. It was a good idea to state in the documentation the expected usage pattern because that was missing in all the new documentation. I.e, even the documentation for org.eclipse.ui.PlatformUI.getDialogSettingsProvider(Bundle) it doesn't hint at how one acquires a "bundle". It makes me wonder why the implementation is different from this where I don't have to know how to get a bundle: public static IPreferenceStore createPreferenceStore(Class<?> clazz) { return new ScopedPreferenceStore(InstanceScope.INSTANCE, FrameworkUtil.getBundle(clazz).getSymbolicName()); } All my screaming and carrying on is 100% about the people who are not doing new things but rather have done things in the past and would like their investments in the Eclipse of the past to continue to be of value for the Eclipse of the future (the better, faster Eclipse that people like you help to create).
I understand that the deprecation is annoying. And I agree that we don't need that deprecation. The usage of the whole AbstractUiPlugin should be discouraged. But there are numerous existing plugins out there that still use them. In pde the default in the plugin wizard was already changed so that people don't create activator although they don't need it. But apart from promoting this in talks and documentation there is not much we can do without offending users. So the deprecation should be removed. That there are still generators out there that create activator and make use of them although there are alternatives, is something that annoys me. Whenever I update existing projects to a more current programming style I have to double check and skip those plugins created by EMF etc. So I am unable to clean up completely because the generators are not updated. And I have to create exceptions in the static analysis as the generated code contains thousands of warnings. Just to give back some "rant" ;) As an open source contributor and committer for several years we obviously all know that we are mostly not payed for what we are doing. Thanks Wim for your contribution and that you will remove the deprecation. It will help cleaning up our code base. I'm sure you would have done it even without the rude comments here.
> All my screaming and carrying on is 100% about the people who are not doing > new things but rather have done things in the past and would like their > investments in the Eclipse of the past to continue to be of value for the It's all good. I don't mind a good rant every now and then, it shows commitment. > It makes me wonder why the implementation > is different from this where I don't have to know how to get a bundle: > > public static IPreferenceStore createPreferenceStore(Class<?> clazz) { > return new ScopedPreferenceStore(InstanceScope.INSTANCE, > FrameworkUtil.getBundle(clazz).getSymbolicName()); I had it like this earlier but then changed the parameter to <bundle>. I figured it made it more clear that the dialog settings were on a per-bundle basis.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/172631 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=dbc60a4e81f6b2eae43e5237e7deb6ef481d93a6
(In reply to Dirk Fauth from comment #43) > That there are still generators out there that create activator and make use > of them although there are alternatives, is something that annoys me. > Whenever I update existing projects to a more current programming style I > have to double check and skip those plugins created by EMF etc. So I am > unable to clean up completely because the generators are not updated. And I > have to create exceptions in the static analysis as the generated code > contains thousands of warnings. > > Just to give back some "rant" ;) > Yes, everything that EMF generates that produces a warning, people will rant about; hence my preemptive ranting about this issue. Even things that must first be configured to be a warning, people will still rant about that too. So one slowly ends up with more and more options to try to make the ranters happy, but then people rant about too many confusing options. Maintaining all the options all gets more costly too. And introducing new options risks regressions; people rant about those too. Lots and lots of ranting. You just can't please everyone. (And that's not ranting about the fact that it's often hard just to get a thank you out of the whole free meal deal.) > As an open source contributor and committer for several years we obviously > all know that we are mostly not payed for what we are doing. > Well, it's hit and miss. There are of course people who are paid full time to work on open source along with many people who work only because it feels good to do so. > Thanks Wim for your contribution and that you will remove the deprecation. Yes, I appreciate that too! Very much!! > It will help cleaning up our code base. I'm sure you would have done it even > without the rude comments here. I'm sorry if I was rude. In my own defense, look at the time frames involved. It completely freaks me out when Sunday morning, very early, I try to figure out why my Oomph development environment just can't launch an installer application with a 2020-12 target platform as I prepare to do a build for 2020-12 M3 in my "spare time". Only to notice a whole whack of new warnings. Then, when I update my Eclipse SDK IDE to understand where these warnings come from, I spend the next 3 hours trying to figure out why the SDK IDE is not linking to the right version of javax.annotation via package imports in some projects but does it correctly in others. At this point, I think very bad thoughts and have very bad feelings about how I am using my "spare time".
Any reason why this bug is still open? I see the code merged and no pending gerrits, so this bug should be resolved/fixed in 4.18. If there are issues with the implementation of the feature (like bug 574502), I would propose to create dedicated bugs for each problem.
FYI, another not nice side effect of this change is this bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=581876 I.e., the installer's persistent state buttons no longer persist. Dialog settings are only persisted when org.eclipse.ui.internal.Workbench.busyClose(boolean) publishes the appropriate event. UIEvents.publishEvent(UIEvents.UILifeCycle.APP_SHUTDOWN_STARTED, application); But the installer has no workbench and is not an MApplication so has no way to post such an event. Given there is no API to save all dialog settings, the solution is not pretty: https://git.eclipse.org/c/oomph/org.eclipse.oomph.git/commit/?id=e935efd6619f7be0b9f5da29f8200617f5f3d732
