[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH] Warn when ignoring BIOS RAID members (#560932)


On 02/17/2010 04:01 PM, Chris Lumens wrote:
Thanks for tackling the base InstallInterface class problem.  It's about
time this got looked at.  I have some generic comments:

* Do you have further patches in the works to move methods and
   attributes into InstallInterfaceBase, or do you want the rest of us to
   fill in the blanks as we have time?

I was sort of hoping for "the rest of us to fill in the blanks as we have time?",
I only added this to make a start with untangling the having 3 interface classes
without a common base mess, as I really couldn't believe I needed to add the exact
same code 3 times.

* I think this patch supercedes the one I sent earlier about making
   command line mode error out on all unknown method calls, as now we can
   just add stubs to InstallInterfaceBase that do the erroring for us.


* When I was thinking about doing this earlier, I was planning on
   creating an intf/ directory and putting the base class in __init__.py,
   then moving parts of gui.py, text.py, and cmdline.py all in there.

This sounds like a good plan in the long run, but I needed something I can
put in RHEL-6 (after beta1), hence this I admit half baked solution.

* We've got a trend now of putting more storage-related elements into
   the UI classes (see the new self._warnUnusedRaidMembers).  In my
   opinion, they don't really fit into the UI classes all that well but
   they don't really fit in the storage classes either.  Anything else
   we can come up with here?

I think the intf class makes sense, we want to avoid asking the same question
multiple times, so it makes sense for the class doing the asking to keep track
of earlier answers (or in this case things already warned about).

This way we can even override the behavior in an interfaceClass and warn every
single time we encounter this (when going back / forth) should some kind of
new interface type call for this, say the PleaseAnnoyMe(InterfaceClassBase)
class :)



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]