[Mageia-dev] NEED REVIEW: drakdisk refresh feature

Mon Oct 15 23:25:08 CEST 2012

Op maandag 15 oktober 2012 15:05:28 schreef Pascal Terjan:
+> On Sat, Oct 13, 2012 at 6:52 PM, AL13N <alien at rmail.be> wrote:
+> > Hi all,
+> > 
+> > This is a first patch, concerning the drakdisk feature
+> > https://wiki.mageia.org/en/Feature:DiskDrakeRedesign
+> > 
+> > the patch does the following:
+> > - group the functionality to actually get all the disks in a function
+> > - pass a ref to the function as the do_force_reload parameter to
+> > interactive - make a Refresh function that returns "force_reload"
+> > - add a button in the gtk
+> > 
+> > what it still needs for the first stage of this feature:
+> > - a button for the curses one
+> > - remove the abort message if no disks are found, but add a text in place
+> > of the disk tabs to say there are no disks and telling you can refresh
+> > after plugging in a disk.
+> > 
+> > 
+> > attached is a small patch for /soft/drakx
+> > 
+> > can *anyone* tell me if i'm on the right track, or if i'm doing ugly
+> > stuff, or if i'm forgetting important parts?
+> 
+> Sorry for not looing at it faster but after one month travelling my
+> home was burgled 10 days ago which mean I could not yet come back to a
+> normal life...
+
+that is no worries, i hope you'll be better soon-ish.
+
+> How is it different from the "Reload partition table" action hidden
+> behind "More" (which for some reason was per disk even if it reloads
+> them all) and only displayed during install?
+> There is nothing obviously wrong, most of the work will be to make
+> sure every part of the code handles this correctly (mostly
+> raid/lvm/...) and interface also recreates tabs correctly etc...
+> It has been broken/fixed various times and if this is to be displayed
+> more visibly it should be properly tested
+
+i realized this afterwards, and i think you're right, perhaps i should handle 
+it a bit better:
+
+-> actively look for new devices (or size-changed devices) and handle those; 
+and thus not change all the changes you've made already.
+
+The point of this is more like stepping stone towards the real things: ie: not 
+aborting if there are no disks. and later adding installable network-based 
+disks (for diskless systems).
+
+And also, if your one of your hotplugged disks isn't correctly in it, i'd like 
+to refresh it, so the 4th one is detected and so i can add it to the raid, for 
+instance. instead of exiting the installer, etc...
+
+in any case, it seems like someone found that it showed gray for a all disks, 
+so it might not be that good after all...
+
+but it would be nice for me though if i could pick your brain about the code 
+and the way of programming though... it's not always easy to get around this 
+kind of code.
+
+Can i ask you in the future to look at this from time to time?
+
+also, i fail to correctly grasp the purpose of the $do_force_reload variable. 
+in some forms, it's used, and some it doesn't. but the function that is 
+supposed to be parsed into that, it not clear what it should return. (to me)
+
+Thanks for the feedback already.
+
+AL13N
+