Susan Sohn wrote: > On 10/09/09 00:47, Evan Layton wrote: >> I need to get reviews for the following bugs that are causing failures >> if the directories where the boot/grub menu files live are missing >> (/rpool/boot and /rpool/boot/grub). Also this fix resolves the issue >> with the add_splash_image_to_grub_menu that was causing a failure to >> create a menu.lst file when it's missing. >> >> >> The bugs: >> http://defect.opensolaris.org/bz/show_bug.cgi?id=7880 >> http://defect.opensolaris.org/bz/show_bug.cgi?id=11436 >> >> >> The webrev: >> http://cr.opensolaris.org/~evanl/7880/ >> >> >> Thanks, >> -evan >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > Hi Evan, > > Here are my comments: > > usr/src/lib/libbe/tbeadm/Makefile > Needs updated copyright
@#%#$ miss that again :-( > > usr/src/lib/libbe/be_utils.c > 3445,3457-3461 Why is temp_str needed here? It doesn't look like it is > used. > I also think you can remove the else clause altogether Yes it's used to strip the menu.lst off the end of menu_path. However with the change that Jack suggested to use dirname() instead the need for this goes away so I'm removing it. > > 3469-3470 - There is a %s for the dirname, but no dirname. fixed... > > usr/src/lib/libict_pymod/ict.py > 305 - durring->during fixed > 329 - maybe append "during install" to error msg? good idea, will do. > 319 - this comment should be back down at 360. right I'll move it > 334,336 Shouldn't the comment be different for these two lines? Yes and I need to add more comments here to detail why we don't want to use the BASEDIR here if we're not running this from an install. > 360 - The code associated with this comment was moved up, so the comment > should also be moved up. Maybe you moved the wrong comment (319) up > by accident. Yes it looks like I made a cut and paste error here. :-( > 882 - You don't really need the backslash, python will know to continue > inside a paren OK this was suggested as part of the PEP8 formatting but I may have that wrong. Keith do you know what the correct thing is here? > > Thanks, > Sue
