Hi Keith, I took a look at UI components + text-install.py (so that I have better picture how things work together). Please see my comments below - just couple of nits.
Thank you, Jan text-install.py --------------- 35 import subprocess Since only call() is used from 'subprocess' Python module, could we just import this one and simplify to call to it - e.g. 35 import subprocess -> 35 from subprocess import call 240,251 subprocess.call -> 240,251 call Ditto for 'platform' module - only processor() is consumed: 33 import platform -> 33 from platform import processor 101 if logname is not None: -> 101 if logname: 122,123 - comment seems incomplete line 196, 233 - since we already switched to 2.6, those comments should be dealt with - e.g. addressed or changed appropriately. base_screen.c ------------- 58,59,60 - just a nit - please add space between 'Installer' word and subsequent question mark 62 - installer -> Installer (to be consistent with rest of the messages) main_window.py -------------- Please add more comments in MainWindow pop_up method in order to make life easier for future maintainers. Keith Mitchell wrote: > Hi Glenn and all other reviewers, > > I have posted an updated webrev at: > http://cr.opensolaris.org/~kemitche/text_v2/ > > > UI Components: > cmd/text-install/osol_install/text_install/action.py > cmd/text-install/osol_install/text_install/base_screen.py* > cmd/text-install/osol_install/text_install/color_theme.py > cmd/text-install/osol_install/text_install/edit_field.py > cmd/text-install/osol_install/text_install/error_window.py > cmd/text-install/osol_install/text_install/inner_window.py > cmd/text-install/osol_install/text_install/list_item.py > cmd/text-install/osol_install/text_install/main_window.py > cmd/text-install/osol_install/text_install/scroll_window.py > cmd/text-install/osol_install/text_install/window_area.py > > * This file is also relevant to the "Screens" group >
