Looks good to me! Alok
On Wed, 24 Feb 2010, Keith Mitchell wrote: > Hi Alok, > > Thanks for the review. See responses below. > > - Keith > > On 02/24/10 01:07 PM, Alok Aggarwal wrote: >> Hi Keith, >> >> Looks good, I mostly have nits. >> >> text-mode-menu.ksh: checkin comment mentions 14372 twice. > > Mostly a merge/commit issue. The changes will be delivered to the text > install gate as multiple changesets (note that when the entire project goes > back to slim_source, *all* changesets will be combined anyway). > >> >> text-mode-menu.ksh: line 40: /usr/xpg4/bin/awk still >> doesn't seem to be used. > > Looks like I forgot to update the webrev. > >> >> text-mode-menu.ksh: why does TSTP need to be blocked? > > It's actually being unblocked. TSTP is blocked by the text-mode-menu.ksh > script; in order to enable bash's job control (and keep bash from complaining > about job control being disabled) when dropping to the shell, the signals > need to be restored during the subprocess spawning. > >> >> edit_field.py: Seems to not have any changes made to it. > > I made some white space changes, which don't get caught by webrev. > > Incremental webrev posted to: > http://cr.opensolaris.org/~kemitche/14372_v2/ > >> >> Do these changes not implement periodic repaint of the >> screen as mentioned in - >> http://defect.opensolaris.org/bz/show_bug.cgi?id=14206#c2 > > That was implemented. See install_progress.py, line 202. > >> >> If so, the bug report should be updated appropriately. >> >> Alok >> >> On Tue, 23 Feb 2010, Keith Mitchell wrote: >> >>> FYI - I've updated the fix for 14372 based on comment 4 of the bug: >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=14372#c4 >>> In the process of testing now, but I don't anticipate any issues as a >>> result. >>> >>> The original webrev has been replaced with the one that contains the >>> updated code. >>> >>> - Keith >>> >>> On 02/23/10 09:24 AM, Keith Mitchell wrote: >>>> Hi all, >>>> >>>> I'd like to request a review for my changes in the text install gate for >>>> bugs 14206 and 14372. >>>> >>>> Webrev: >>>> http://cr.opensolaris.org/~kemitche/14206.14372/ >>>> >>>> Bugs: >>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=14206 >>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=14372 >>>> >>>> Summary: >>>> - Updated text-mode-menu.ksh to read /etc/passwd to spawn the "Shell" >>>> option based on the user's login-shell >>>> - Updated main_window.py to repaint the screen when ctrl-L is pressed; >>>> streamlined different places in the code so that all calls to "getch" >>>> flow through main_window.py (previously there was some scattering of code >>>> there) >>>> >>>> Testing done: >>>> For 14372: Copied the text-mode-menu.ksh changes to a system booted with >>>> the text installer image. Changed the SMF start/exec property to point at >>>> the modified menu, and restarted the service. Verified that the shell >>>> spawned matched what was defined as root's login shell. >>>> >>>> For 14206: Temporarily added code that distorts the screen at regular >>>> intervals; verified that ctrl-L would repaint the entire screen in all >>>> cases (while on a text only screen, while in an editable field, while >>>> selecting an item from a list, while a pop-up window was active, and >>>> during the installation itself). >>>> >>>> Additional note: >>>> I updated disk.c, partition.c, and slice.c to quiet some build warnings >>>> nightly was giving. There is no bug for those changes, but if there's >>>> concern about the changes here, I will file a bug and address the issue >>>> separately. >>>> >>>> Thanks, >>>> Keith >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> >
