Community
Participate
Working Groups
This is manifestly a user interface type. There are some references to it from the core system registry for creating hosts. This core system registry interface should be modified to, perhaps, take objects of a more general type that may be attached to hosts as creation. The wizard pages may then be moved to the UI where they belong.
Not for 2.0
Created attachment 95210 [details] First patch at refactoring This moves and renames some Interfaces. In particular, ISystemNewConnectionWizardPage is moved to the UI plugin, but a new interface is defined in core for carrying the properties for new subsystems. ISystemNewConnectionWizardPage extends this new interface. This is a fairly straightforward refactoring. I'd like to see it approved for M6 if possible, but I will place it up to vote since we are at the cutoff point.
Created attachment 95211 [details] mylyn/context/zip
Created attachment 95212 [details] Second patch Some changes to SystemView made it into the last patch by mistake. This one is the correct content.
Created attachment 95213 [details] mylyn/context/zip
Hm... while the change might work fine from a technical point of view, it does look a little odd at places. For instance, I'm not quite understanding: * Why are there separate ISystemNewSubSystemProperties and ISystemNewServiceSubSystemProperties interfaces? I thought we had done away with the distinciton between SubSystem and ServiceSubSystem. * From a *Properties interface I'd usually expect methods like getValue(), setValue() and the like. But it looks like it's the other way round - the class implementing this inteface is capable of configuring an ISubSystem, right? So then I'd propose renaming the interface to ISubSystemConfigurator for instance. Adding more class Javadoc about how the interface is intended to be used would probably help me better understand this. * In RSEDefaultNewConnectionWizard, method protected ISystemNewSubSystemProperties getFirstAdditionalPage() { really looks odd and not understandable. It's better when the method returns an ISubSystemConfigurator but still a little bit odd, since the concepts of a WizardPage and the Configurator are mixed. I think that this method should continue returning an ISystemNewConnectionWizardPage.
(In reply to comment #6) > Hm... while the change might work fine from a technical point of view, it does > look a little odd at places. For instance, I'm not quite understanding: > > * Why are there separate ISystemNewSubSystemProperties and > ISystemNewServiceSubSystemProperties interfaces? I thought we had done away > with the distinciton between SubSystem and ServiceSubSystem. This is a division of labor that existed prior to the refactoring. I'll see if I can get rid of it. > > * From a *Properties interface I'd usually expect methods like getValue(), > setValue() and the like. But it looks like it's the other way round - the > class implementing this inteface is capable of configuring an ISubSystem, > right? So then I'd propose renaming the interface to > ISubSystemConfigurator > for instance. Adding more class Javadoc about how the interface is intended > to be used would probably help me better understand this. Excellent idea. The javadoc was left out for a bit to accomodate just this kind of thing. > > * In RSEDefaultNewConnectionWizard, method > protected ISystemNewSubSystemProperties getFirstAdditionalPage() { > really looks odd and not understandable. It's better when the method > returns an ISubSystemConfigurator but still a little bit odd, since > the concepts of a WizardPage and the Configurator are mixed. I think > that this method should continue returning an ISystemNewConnectionWizardPage. Yes, you are correct. I'll fix that. Its an artifact of how I went about doing the refactoring.
Committing.
Summary of API Changes: ----------------------- * ISystemNewConnectionWizardPage moved from rse.core to rse.ui --> Clients need to "Organize Imports" * ISubSystemConfigurator interface introduced in rse.core as a base interface of ISystemNewConnectionWizardPage --> Especially non-UI clients should consider using ISubSystemConfigurator rather than ISystemNewConnectonWizardPage * ISubSystemPropertiesWizardPage removed (replaced by ISubSystemConfigurator) --> Clients need to rename any usage of ISubSystemPropertiesWizardPage * AbstractSystemNewConnectionWizardPage implements ISubSystemConfigurator --> All extenders of that class MUST implement new applyValues() method * Following methods now take ISubSystemConfigurator rather than ISystemNewConnectionWizardPage. Since the former is a base interface of the latter, this change is source compatible for clients (no changes required for clients, but extenders need to adjust their method signatures): - ISubSystemConfiguration#createSubSystem() - ISystemRegistry#createHost()
