[yast-devel] yast Mode.* variables
Having just learned about the Mode.autoupgrade variable I did some research and found a total of 12 instances of Mode.* (maybe I missed some): Mode.autoinst Mode.autoupgrade Mode.commandline Mode.config Mode.installation Mode.live_installation Mode.normal Mode.repair Mode.screen_shot Mode.test Mode.testsuite Mode.update I think they should not only be prominently documented but preferably the number should be reduced. They are a nuisance and one of the reasons yast is so hard to debug. There's always some use case with some unexpected combination of them. This just asks for trouble. IMO Steffen -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Tue, Jul 22, 2014 at 11:54:23AM +0200, Steffen Winterfeldt wrote:
Having just learned about the Mode.autoupgrade variable I did some research and found a total of 12 instances of Mode.* (maybe I missed some):
I think they should not only be prominently documented but preferably the number should be reduced.
They are a nuisance and one of the reasons yast is so hard to debug. There's always some use case with some unexpected combination of them. This just asks for trouble.
I second that. When I was fixing some bugs for the yast2-s390
module it was (and still is) unclear to me what combination of
Mode and Stage settings are possible and how e.g. the Read,
Write, Import, Export functions should behave depending on the
settings.
ciao
Arvin
--
Arvin Schnell,
On 07/22/2014 12:12 PM, Arvin Schnell wrote:
On Tue, Jul 22, 2014 at 11:54:23AM +0200, Steffen Winterfeldt wrote:
Having just learned about the Mode.autoupgrade variable I did some research and found a total of 12 instances of Mode.* (maybe I missed some):
I think they should not only be prominently documented but preferably the number should be reduced.
They are a nuisance and one of the reasons yast is so hard to debug. There's always some use case with some unexpected combination of them. This just asks for trouble.
I second that. When I was fixing some bugs for the yast2-s390 module it was (and still is) unclear to me what combination of Mode and Stage settings are possible and how e.g. the Read, Write, Import, Export functions should behave depending on the settings.
I think that we need that many modes. Just look at the control file, you will find different workflow for most of them and the mode is there to select the right one. Therefore, when AutoYaST upgrade (with a new work-flow) was introduced, it was needed to also introduce a new mode. And I think that the same will happen if another kind of work-flow is introduced. There are wrappers in Mode.rb, which aggregate the individual modes (e.g. Mode.install is true for live install as well as autoinstall, in case you don't care). The other question is whether we don't rely too much on the information about the mode, perhaps there are other ways to find out? In any case, I agree that documentation should be improved. Jiri -- Regards, Jiri Srain Project Manager --------------------------------------------------------------------- SUSE LINUX, s.r.o. e-mail: jsrain@suse.com Lihovarska 1060/12 tel: +420 284 084 659 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.com -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Tue, 22 Jul 2014, Jiri Srain wrote:
I think that we need that many modes. Just look at the control file, you will find different workflow for most of them and the mode is there to select the right one. Therefore, when AutoYaST upgrade (with a new work-flow) was introduced, it was needed to also introduce a new mode. And I think that the same will happen if another kind of work-flow is introduced.
One thing about the Mode vraiable is that it mixes different things. There (a) are kinds of workflows: Mode.autoinst Mode.autoupgrade Mode.installation Mode.live_installation Mode.normal Mode.repair Mode.update and (b) flags, modifying workflows: Mode.commandline Mode.config Mode.screen_shot Mode.test Mode.testsuite The first group's modes are basically mutually exclusive if I understand you correctly (wew workflow -> new mode). Then this should be reflected in the code. So instead we should have something like context = autoinst|autoupgrade|... Then there's only 7 cases to deal with instead of 2^7. You can always create a wrapper object for convenience. Like context.auto? or context.install?. Steffen -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On 07/22/2014 01:13 PM, Steffen Winterfeldt wrote:
On Tue, 22 Jul 2014, Jiri Srain wrote:
I think that we need that many modes. Just look at the control file, you will find different workflow for most of them and the mode is there to select the right one. Therefore, when AutoYaST upgrade (with a new work-flow) was introduced, it was needed to also introduce a new mode. And I think that the same will happen if another kind of work-flow is introduced.
One thing about the Mode vraiable is that it mixes different things. There (a) are kinds of workflows:
Mode.autoinst Mode.autoupgrade Mode.installation Mode.live_installation Mode.normal Mode.repair Mode.update
and (b) flags, modifying workflows:
Mode.commandline Mode.config Mode.screen_shot Mode.test Mode.testsuite
The first group's modes are basically mutually exclusive if I understand you correctly (wew workflow -> new mode). Then this should be reflected in the code. So instead we should have something like
context = autoinst|autoupgrade|...
Then there's only 7 cases to deal with instead of 2^7. You can always create a wrapper object for convenience. Like context.auto? or context.install?.
There is a single variable which stores the mode. All the rest are just wrappers comparing this variable to a particular value (or values). Therefore you definitely cannot have 2^7 modes. (*) The confusion here is IMO the result of conversion from YCP; if the code used ruby naming conventions, Mode.update would definitely be called Mode.update? # we're doing an update def update mode == "update" || mode == "autoupgrade" end (*) OK, there is one variable for general mode, one variable for testing mode (which should IMO be squeezed out as a next step after switch to Ruby) and one variable for UI mode - but still not 3^7 different modes :-) Jiri -- Regards, Jiri Srain Project Manager --------------------------------------------------------------------- SUSE LINUX, s.r.o. e-mail: jsrain@suse.com Lihovarska 1060/12 tel: +420 284 084 659 190 00 Praha 9 fax: +420 284 084 001 Czech Republic http://www.suse.com -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Tue, 22 Jul 2014, Jiri Srain wrote:
There is a single variable which stores the mode. All the rest are just wrappers comparing this variable to a particular value (or values). Therefore you definitely cannot have 2^7 modes. (*)
The confusion here is IMO the result of conversion from YCP; if the code used ruby naming conventions, Mode.update would definitely be called Mode.update?
# we're doing an update def update mode == "update" || mode == "autoupgrade" end
(*) OK, there is one variable for general mode, one variable for testing mode (which should IMO be squeezed out as a next step after switch to Ruby) and one variable for UI mode - but still not 3^7 different modes :-)
Ok, looking at the actual Mode implementation sheds some light on this. I'd prefer not having to do this for every class, though. :-/ So there are 8 generic 'modes' and some test functions combining these. Roughly along the lines I guessed in my post. Some issues: 1) the active mode is not logged; at least not during the usual install workflow because Initialize() doesn't use SetMode() to set the mode; SetTest() and SetUI() don't even bother 2) it's really anyone's guess why Mode.update combines update and autoupgrade while Mode.autoinst does not combine autoinstallation and autoupgrade 3) there actually 3 mode categories: workflow modes, test modes, and ui modes... Steffen -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Tue, Jul 22, 2014 at 05:45:50PM +0200, Steffen Winterfeldt wrote:
Ok, looking at the actual Mode implementation sheds some light on this. I'd prefer not having to do this for every class, though. :-/
You are right. Here you are: https://github.com/yast/yast-yast2/pull/272 -- Martin Vidner, Cloud & Systems Management Team http://en.opensuse.org/User:Mvidner Kuracke oddeleni v restauraci je jako fekalni oddeleni v bazenu
Dne 22.7.2014 11:54, Steffen Winterfeldt napsal(a):
Having just learned about the Mode.autoupgrade variable I did some research and found a total of 12 instances of Mode.* (maybe I missed some):
Mode.autoinst Mode.autoupgrade Mode.commandline Mode.config Mode.installation Mode.live_installation Mode.normal Mode.repair Mode.screen_shot Mode.test Mode.testsuite Mode.update
I think they should not only be prominently documented but preferably the number should be reduced.
I agree with reduction, I think these could be possibly removed: Mode.repair - the repair module has been dropped completely, this mode is dead and does not make sense anymore, it can be safely removed [This sounds like a junior job, Ancor, Christopher, what do you think about it? ;-)] Mode.test, Mode.testsuite - I do not know the exact difference between the two, but after switching to Ruby and Rspec we can easily mock any call so we should do not need these workarounds. The only problem is with the old tests, but they should be (gradually) rewritten to Rspec so eventually we could get rid of these modes... Mode.screen_shot - used only at few places, I guess nobody use it or even know that it exists or how to invoke it. It should be dropped. Mode.commandline - could be also dropped but it might need some essential changes in the Yast architecture, but after switch to Ruby it should not be needed at all... The easiest to drop are Mode.repair and Mode.screen_shot, I think it's safe to remove them even now when we are in SLE12 RC phase... -- Ladislav Slezák Appliance department / YaST Developer Lihovarská 1060/12 190 00 Prague 9 / Czech Republic tel: +420 284 028 960 lslezak@suse.com SUSE -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Wed, 23 Jul 2014 16:23:30 +0200
Ladislav Slezak
Dne 22.7.2014 11:54, Steffen Winterfeldt napsal(a):
Having just learned about the Mode.autoupgrade variable I did some research and found a total of 12 instances of Mode.* (maybe I missed some):
Mode.autoinst Mode.autoupgrade Mode.commandline Mode.config Mode.installation Mode.live_installation Mode.normal Mode.repair Mode.screen_shot Mode.test Mode.testsuite Mode.update
I think they should not only be prominently documented but preferably the number should be reduced.
I agree with reduction, I think these could be possibly removed:
Mode.repair - the repair module has been dropped completely, this mode is dead and does not make sense anymore, it can be safely removed
[This sounds like a junior job, Ancor, Christopher, what do you think about it? ;-)]
Mode.test, Mode.testsuite - I do not know the exact difference between the two, but after switching to Ruby and Rspec we can easily mock any call so we should do not need these workarounds. The only problem is with the old tests, but they should be (gradually) rewritten to Rspec so eventually we could get rid of these modes...
Difference is that Mode.test include also "test" which is "demo". It was ancient mode to demonstrate yast ability, but with some differences. I think we can drop it. Josef
Mode.screen_shot - used only at few places, I guess nobody use it or even know that it exists or how to invoke it. It should be dropped.
Mode.commandline - could be also dropped but it might need some essential changes in the Yast architecture, but after switch to Ruby it should not be needed at all...
The easiest to drop are Mode.repair and Mode.screen_shot, I think it's safe to remove them even now when we are in SLE12 RC phase...
--
Ladislav Slezák Appliance department / YaST Developer Lihovarská 1060/12 190 00 Prague 9 / Czech Republic tel: +420 284 028 960 lslezak@suse.com SUSE
-- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Tue, 22 Jul 2014 11:54:23 +0200 (CEST)
Steffen Winterfeldt
Having just learned about the Mode.autoupgrade variable I did some research and found a total of 12 instances of Mode.* (maybe I missed some):
Mode.autoinst Mode.autoupgrade Mode.commandline Mode.config Mode.installation Mode.live_installation Mode.normal Mode.repair Mode.screen_shot Mode.test Mode.testsuite Mode.update
I think they should not only be prominently documented but preferably the number should be reduced.
They are a nuisance and one of the reasons yast is so hard to debug. There's always some use case with some unexpected combination of them. This just asks for trouble.
IMO
Steffen
I agree that there is too much modes and it is quite confusing. For me it make sense to merge stages and modes and have these ones flags: - first_stage? ( is it first stage ) - second_stage? - system? ( not first nor second stage, something like mode normal ) - installation? - update? - automatic? ( all modes that use control file to pass some configuration so autoinstallation and autoupgrade ) - export? ( indicates that we only export autoyast profile, but for me it indicates that code is wrongly organized if it need it ) - commandline? - testsuite? ( we should drop it in future as old testsuite is horrible ). Josef Josef -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
participants (6)
-
Arvin Schnell -
Jiri Srain -
Josef Reidinger -
Ladislav Slezak -
Martin Vidner -
Steffen Winterfeldt