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