legacy strategy and other things
Hi Ancor, [ https://github.com/yast/yast-storage-ng/pull/47 ] working on the legacy strategy I stumbled across some general (design) issues: (1) the current 'refinement' using Modules does not work and is somewhat clumsy; is there anything against extending the libstorage-ng classes directly (like I did in storage/refinements.rb? - and (1a) dump the refinements directory entirely? (2) the internal structure of yast-storage-ng is a nightmare - can we do something about it? always having to require a bunch of sub-classes is pointless; ruby ends up including basically all of them anyway; see boot_requirements_strategies/base.rb for what I mean. As always all strategies are included anyway, we can bundle the requires in the base class. Or is this so un-ruby? (3) we require 'yast' just for the logger in different places; can we stuff this into a single place and include our own (wrapper) logger instead? (4) what will an external user of storage-ng 'require'? Steffen -- Give orange me give eat orange me eat orange give me eat orange give me you. (chimp Nim, using sign language) -- To unsubscribe, e-mail: opensuse-storage+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-storage+owner@opensuse.org
On 04/14/2016 05:11 PM, Steffen Winterfeldt wrote:
Hi Ancor,
[ https://github.com/yast/yast-storage-ng/pull/47 ]
working on the legacy strategy I stumbled across some general (design) issues:
(1) the current 'refinement' using Modules does not work and is somewhat clumsy;
Can you please elaborate what you mean with "does not work"? For the purpose they are being used in the existing code, they work quite well.
is there anything against extending the libstorage-ng classes directly (like I did in storage/refinements.rb? - and (1a) dump the refinements directory entirely?
Direct monkey patching in Ruby is considered dangerous. For example, it pollutes the classes all over the execution, while refinements only modify the classes in the environment which has explicitly enabled the refinement. In other words, there is no way to turn off or isolate the monkey patches if you do them directly. With refinements, if you try to patch a class that is still not known (let's say you forgot a "require") it will raise an error. With wild monkey patching you would be indeed defining the class for the first time, with hard to predict results. When a error occurs, is usually harder to find/debug with direct monkey patching. If you google a bit you will probably find other reasons. I just feel more confident using refinements than with wild monkey patching and it's working just fine for me so far.
(2) the internal structure of yast-storage-ng is a nightmare - can we do something about it? always having to require a bunch of sub-classes is pointless; ruby ends up including basically all of them anyway; see boot_requirements_strategies/base.rb for what I mean. As always all strategies are included anyway, we can bundle the requires in the base class. Or is this so un-ruby?
Sure we can. Actually we already have src/lib/storage/boot_requirements_strategies.rb as a way to require all strategies at once and there is nothing against moving those "require" that are repeated in all strategies to base.rb.
(3) we require 'yast' just for the logger in different places; can we stuff this into a single place and include our own (wrapper) logger instead?
Sure, why not?
(4) what will an external user of storage-ng 'require'?
It depends on which parts it wants to use. For example, to use the proposal, require "storage/proposal" settings Yast::Storage::Proposal::Settings.new Yast::Storage::Proposal.new(settings) -- Ancor González Sosa YaST Team at SUSE Linux GmbH -- To unsubscribe, e-mail: opensuse-storage+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-storage+owner@opensuse.org
On Thursday 2016-04-14 17:48, Ancor Gonzalez Sosa wrote:
On 04/14/2016 05:11 PM, Steffen Winterfeldt wrote:
(1) the current 'refinement' using Modules does not work and is somewhat clumsy;
Can you please elaborate what you mean with "does not work"? For the purpose they are being used in the existing code, they work quite well.
To add a useful #inspect/#to_s you have to modify the class itself. But from your comment below I see that hits a sore point in the ruby world.
I just feel more confident using refinements than with wild monkey patching and it's working just fine for me so far.
Currently, you have to remember to activate all kinds of refinements all over the code. While this may count as defensive programming its also rather tedious.
(4) what will an external user of storage-ng 'require'?
It depends on which parts it wants to use. For example, to use the proposal,
require "storage/proposal" settings Yast::Storage::Proposal::Settings.new Yast::Storage::Proposal.new(settings)
I would have expected require "yast/storage". Why do we install to /usr/share/YaST2/lib und not /usr/lib64/ruby anyway? Steffen -- To unsubscribe, e-mail: opensuse-storage+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-storage+owner@opensuse.org
On 04/15/2016 11:16 AM, Steffen Winterfeldt wrote:
On Thursday 2016-04-14 17:48, Ancor Gonzalez Sosa wrote:
On 04/14/2016 05:11 PM, Steffen Winterfeldt wrote:
(1) the current 'refinement' using Modules does not work and is somewhat clumsy;
Can you please elaborate what you mean with "does not work"? For the purpose they are being used in the existing code, they work quite well.
To add a useful #inspect/#to_s you have to modify the class itself. But from your comment below I see that hits a sore point in the ruby world.
About #inspect/#to_s https://github.com/yast/yast-storage-ng/pull/47#discussion_r59836546
I just feel more confident using refinements than with wild monkey patching and it's working just fine for me so far.
Currently, you have to remember to activate all kinds of refinements all over the code. While this may count as defensive programming its also rather tedious.
(4) what will an external user of storage-ng 'require'?
It depends on which parts it wants to use. For example, to use the proposal,
require "storage/proposal" settings Yast::Storage::Proposal::Settings.new Yast::Storage::Proposal.new(settings)
I would have expected require "yast/storage".
From the code/files point of view this translates into
Well, there are 2 topics here. 1) Nesting require "storage" # provides ::Storage require "storage/proposal" # provides ::Yast::Storage::Proposal This is surprising... not to say plain wrong. It would be more consistent if the second would be require "yast/storage/proposal" But that's how it is done in the rest of YaST. Usually "require 'foo' brings Yast::Foo. I'm fine if yast-storage-ng is the first module that introduces "yast" in the path. We should open the discussion in yast-devel, though. 2) One require per functionality and one to fetch them all It's kind of a pattern in Ruby that libraries offer something like require "mylibrary/functionality1" # That would be proposal in our case require "mylibrary/functionality2" # Whatever comes next And, in addition, they offer require "mylibrary" # This drags all possible functionalities in lib/mylibrary/* -> classes lib/mylibrary.rb -> file with just a collection of requires like require "mylibrary/functionality1" require "mylibrary/functionality2" I would not like to loose the opportunity to have one require per functionality (i.e. the current "storage/proposal"). But if we solve the nesting problem, I'm fine (even happy) with having "yast/storage" as a drag-everything-it require.
Why do we install to /usr/share/YaST2/lib und not /usr/lib64/ruby anyway?
I don't know. To be honest. Cheers. -- Ancor González Sosa YaST Team at SUSE Linux GmbH -- To unsubscribe, e-mail: opensuse-storage+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-storage+owner@opensuse.org
participants (2)
-
Ancor Gonzalez Sosa -
Steffen Winterfeldt