Karen, Thanks...
You are correct, Dave has pointed it out also, I am incorrectly writing the "END BOOTADM" at the end of the file. I'll fix that. Huge Thanks! Joe Karen Tung wrote: > Hi Joe, > > Please see my responses below. > > Joseph J VLcek wrote: >> Karen, >> >> Thank you for your feedback. I appreciate it. >> >> Karen Tung wrote: >>> Hi Joe, >>> >>> I am having trouble following the code as it is written, even with the >>> comments. At least, the comments need to be improved. From my >>> understanding of the problem that need to be solved, and reading the >>> existing code many times, I *think* the code is doing the following. >>> >>> -------------- >>> Find the first entry in the menu.lst file that boots the installed >>> rpool, >>> and enable Happy Face Boot for that entry. The original content of >>> the entry will be saved to create an additional entry that doesn't >>> have Happy Face Boot enabled. Comments and white space from the >>> original entry will not be included in the additional entry. If more >>> than >>> one entry exist in the original menu.lst file, only the first entry will >>> be modified to enable Happy Face Boot. >>> -------------- >> >> That is correct. I can add similar text as a comment. >> >> >>> >>> Other comments I have about the code. >>> >>> 1) You don't need to have 2 "try". You can just have 1 try with with >>> multiple "except" >>> and the "finally" >> >> I check with Jack Schwartz on this. I turns out for python 2.4 the >> extra level is needed. >> > I just wrote a little test program to check. You are right that it does > need the extra level. I am still surprised that we need the extra level. > I guess I was thinking about Java. >> >>> >>> 2) You seem to use "END BOOTADM" as the indication of end of the >>> entry. I don't think >>> that's a good idea, since that's just a comment. I think one entry >>> should go from one >>> title line to the next title line. >> >> "END BOOTADM" is not being used as the entry indication. The comment >> line containing "END BOOTADM" is saved and then rewritten to the end >> of the file. >> > end of the file? I guess that's where I got confused. I assume that > you just write it to the end of > the happy face boot entry. Why write it to end of the file? >> The title line is being used to indicate the next entry. >> >> >>> >>> 3) The way it is coded now, the "text boot" entry will be the last >>> one in the menu.lst >>> instead of immediately following the line that we have enabled Happy >>> Face Boot. >>> Is that the desired behavior? I would think that it makes more sense >>> to have it >>> immediately follow the Happy Face boot line. >> >> I purposely put the "text boot" entry at the bottom. >> >> >> Currently there is only one entry. I had initially coded this fix to >> rely on that but I felt it would be safer to make the code a little >> more robust. >> >> In the near future the extra "text boot" entry will not be needed >> because support to interrupt the graphical boot using keyboard input >> will provide visibility to the boot messages. >> > OK, this makes sense. > > Thanks, > > --Karen > >> >>> >>> Thanks, >>> >>> --Karen >> >> >> Huge thanks Karen! >> >> Joe >> >> >>> >>> >>> Joseph J VLcek wrote: >>>> Hello, >>>> >>>> Can two people please do a code review for bug: >>>> >>>> 4673 Grub text only mode is required >>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4673 >>>> >>>> >>>> The webrev is available at: >>>> http://cr.opensolaris.org/~joev/bug4673/ >>>> >>>> This fix involves a rewrite of ICT enable_happy_face_boot >>>> >>>> I also fixed some begin and end comment block issues that were >>>> causing vim to display some of the code in the color of a comment. >>>> >>>> * The modules affected and tested: >>>> >>>> ict.py >>>> >>>> * Testing done on logic >>>> >>>> I wrote a stand alone wrapper around the changes so they could be >>>> exercised outside of an installation environment. >>>> >>>> I tested: >>>> >>>> - a menu.lst file with multiple entries >>>> - a menu.lst file with a single entry >>>> - a menu.lst file with embedded comments >>>> - a non-existing menu.lst file, reports an error >>>> - a read only menu.lst files, reports an error >>>> >>>> * Testing done for GUI Install >>>> >>>> I booted a 101 live Image and used mount -F lofs to applied the >>>> updated version of ict.py >>>> >>>> After the installation I performed reboots selecting the default >>>> before the timeout, I let the timeout select the default and I >>>> selected the text boot entry >>>> >>>> * Results: >>>> >>>> The installation completed successfully and the menu.lst file >>>> contained the default graphical boot entry and a second, text boot >>>> entry and both entries provide the expected behavior. >>>> >>>> * Testing done for AI >>>> >>>> No AI testing was performed. >>>> >>>> >>>> Thank you, >>>> Joe >>>> >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >>> >> >
