Keith, Sue,

I have re-reviewed the files in the "UI Components" section.
I have confirmed that all the issues raised previously have
been addressed satisfactorily.

I have only a few minor further comments to make:


cmd/text-install/osol_install/text_install/error_window.py:
   76         logging.debug("displaying err '%s'" % text)
You don't need to interpolate substitution variables before
calling logging.debug, you can pass them in as params, eg:
   76         logging.debug("displaying err '%s'", text)
In fact, pylint usually gives a warning if you use the former,
so presumably the latter is the preferred way.
This also applies to several logging.debug calls in
inner_window.py and scroll_window.py.


cmd/text-install/osol_install/text_install/main_window.py:
  109 #        central_border_area = WindowArea(win_size_y - 4, win_size_x - 2, 
2, 1)
  110 #        self.central_border = InnerWindow(central_border_area,
  111 #                                          color_theme=self.theme)
  112 #        central_win_area = WindowArea(win_size_y - 4, win_size_x - 6, 2, 
3)
You've added some commented-out code.  It should be used
or deleted.


cmd/text-install/osol_install/text_install/scroll_window.py:
   99             self.window.vline(0, 0, ord(" "), self.area.scrollable_lines)
Can you use InnerWindow.BKGD_CHAR instead of ord(" ")?



- Dermot




On 02/04/10 23:00, Susan Sohn wrote:
> The incremental webrev for the text installer project has been posted at:
> http://cr.opensolaris.org/~kemitche/text_v3_incremental/
> 
> This webrev shows the diffs from the original code review with certain
> exceptions as described in the NOTE below.
> 
> The full webrev to date is located at:
> http://cr.opensolaris.org/~kemitche/text_v3/
> 
> We would like to get feedback on this round of changes no later than COB
> Tuesday, 2/9, if at all possible.  If you would like to review these 
> changes but
> can't be done with your comments by then please contact me and we'll work
> something out.
> 
> Thanks,
> Sue
> 
> 
> NOTE:
> For the following files, an incremental webrev was not easily available, 
> so all
> changes are shown:
> README
> cmd/Makefile
> cmd/Makefile.cmd
> cmd/Makefile.targ
> cmd/distro_const/auto_install/ai_sparc_image.xml
> cmd/distro_const/auto_install/ai_x86_image.xml
> cmd/distro_const/utils/Makefile
> cmd/distro_const/utils/grub_setup.py
> cmd/slim-install/finish/install-finish
> cmd/slim-install/svc/media-fs-root
> lib/Makefile
> lib/libtd/td_api.h
> lib/libtd/td_dd.c
> lib/libtd/test_td.c
> pkgdefs/SUNWdistro-const/prototype_com
> 
> --------
> File Groupings:
> 
> Text Install python core:
> cmd/text-install/osol_install/text_install/__init__.py
> cmd/text-install/osol_install/text_install/disk_window.py
> cmd/text-install/osol_install/text_install/screen_list.py
> cmd/text-install/osol_install/text_install/text_install.py
> cmd/text-install/osol_install/text_install/text-install.py
> cmd/text-install/osol_install/text_install/ti_install.py
> cmd/text-install/osol_install/text_install/ti_install_utils.py
> 
> Image/Distro-const:
> cmd/distro_const/auto_install/ai_generic_live.xml
> cmd/distro_const/auto_install/ai_sparc_image.xml
> cmd/distro_const/auto_install/ai_x86_image.xml
> cmd/distro_const/slim_cd/Makefile
> cmd/distro_const/slim_cd/all_lang_slim_cd_x86.xml
> cmd/distro_const/slim_cd/slim_cd_x86.xml
> cmd/distro_const/slim_cd/slimcd_generic_live.xml
> cmd/distro_const/text_install/Makefile
> cmd/distro_const/text_install/text_mode_sparc.xml
> cmd/distro_const/text_install/text_mode_x86.xml
> cmd/distro_const/text_install/tm_gen_cd_content
> cmd/distro_const/text_install/tm_generic_live.xml
> cmd/distro_const/text_install/tm_pre_boot_archive_pkg_image_mod
> cmd/distro_const/utils/Makefile
> cmd/distro_const/utils/common_generic_live.xml
> cmd/distro_const/utils/gen_cd_content 
> cmd/distro_const/slim_cd/slimcd_gen_cd_content
> cmd/distro_const/utils/grub_setup.py
> cmd/distro_const/utils/plat_setup.py
> cmd/distro_const/utils/post_boot_archive_pkg_image_mod_custom
> cmd/slim-install/config/set_lang
> cmd/slim-install/finish/install-finish
> cmd/slim-install/svc/media-fs-root
> cmd/text-install/svc/text-mode-menu.ksh
> cmd/text-install/text-mode-menu/text-mode-menu.ksh
> cmd/text-install/svc/text-mode-menu.xml
> 
> Installation Profile:
> cmd/text-install/osol_install/profile/disk_info.py
> cmd/text-install/osol_install/profile/disk_space.py
> cmd/text-install/osol_install/profile/install_profile.py
> cmd/text-install/osol_install/profile/ip_address.py
> cmd/text-install/osol_install/profile/network_info.py
> cmd/text-install/osol_install/profile/partition_info.py
> cmd/text-install/osol_install/profile/disk_info.py
> cmd/text-install/osol_install/profile/slice_info.py
> cmd/text-install/osol_install/profile/disk_info.py
> cmd/text-install/osol_install/profile/system_info.py
> cmd/text-install/osol_install/profile/user_info.py
> cmd/text-install/osol_install/text_install/Makefile
> 
> 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/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
> 
> Screens:
> cmd/text-install/osol_install/text_install/date_time.py
> cmd/text-install/osol_install/text_install/disk_selection.py
> cmd/text-install/osol_install/text_install/fdisk_partitions.py
> cmd/text-install/osol_install/text_install/help_screen.py
> cmd/text-install/osol_install/text_install/inner_window.py
> cmd/text-install/osol_install/text_install/install_progress.py
> cmd/text-install/osol_install/text_install/install_status.py
> cmd/text-install/osol_install/text_install/log_viewer.py
> cmd/text-install/osol_install/text_install/network_nic_configure.py
> cmd/text-install/osol_install/text_install/network_nic_select.py
> cmd/text-install/osol_install/text_install/network_type.py
> cmd/text-install/osol_install/text_install/partition_edit_screen.py
> cmd/text-install/osol_install/text_install/summary.py
> cmd/text-install/osol_install/text_install/timezone.py
> cmd/text-install/osol_install/text_install/timezone_locations.py
> cmd/text-install/osol_install/text_install/timezone_regions.py
> cmd/text-install/osol_install/text_install/users.py
> cmd/text-install/osol_install/text_install/welcome.py
> 
> Library Changes:
> lib/libict_pymod/ict.py
> lib/libtarget_pymod/Makefile
> lib/libtarget_pymod/discover.c
> lib/libtarget_pymod/disk.c
> lib/libtarget_pymod/disk.h
> lib/libtarget_pymod/geometry.c
> lib/libtarget_pymod/geometry.h
> lib/libtarget_pymod/instantiate.c
> lib/libtarget_pymod/instantiate.h
> lib/libtarget_pymod/mapfile-vers
> lib/libtarget_pymod/partition.c
> lib/libtarget_pymod/partition.h
> lib/libtarget_pymod/slice.c
> lib/libtarget_pymod/slice.h
> lib/libtarget_pymod/tgt.c
> lib/libtarget_pymod/tgt.h
> lib/libtarget_pymod/zpool.c
> lib/libtarget_pymod/zpool.h
> lib/libtd/Makefile
> lib/libtd/td_api.h
> lib/libtd/td_dd.c
> lib/libtd/td_dd.h
> lib/libtd/test_td.c
> lib/libti/ti_api.h
> lib/libtransfer/transfer_mod.py
> lib/libzoneinfo_pymod/Makefile
> lib/libzoneinfo_pymod/libzoneinfo.c
> 
> Packaging/Makefiles: Package definitions, dependencies, makefiles, and
> so forth
> Makefile.master
> README
> Targetdirs
> cmd/Makefile
> cmd/Makefile.cmd
> cmd/Makefile.targ
> cmd/distro_const/Makefile
> cmd/distro_const/auto_install/Makefile
> cmd/text-install/Makefile
> cmd/text-install/osol_install/profile/Makefile
> cmd/text-install/svc/Makefile
> cmd/text-install/text-mode-menu/Makefile
> lib/Makefile
> lib/libict/Makefile
> pkgdefs/Makefile
> pkgdefs/SUNWdistro-const/prototype_com
> pkgdefs/SUNWinstall/prototype_com
> pkgdefs/SUNWtext-install/Makefile
> pkgdefs/SUNWtext-install/depend
> pkgdefs/SUNWtext-install/pkginfo.tmpl
> pkgdefs/SUNWtext-install/prototype_com
> pkgdefs/SUNWtext-install/prototype_i386
> pkgdefs/SUNWtext-install/prototype_sparc
> tools/env/install.pylintrc
> 
> Other: (Help Files, miscellaneous)*
> cmd/text-install/helpfiles/Makefile
> cmd/text-install/helpfiles/date_time.txt
> cmd/text-install/helpfiles/disks.txt
> cmd/text-install/helpfiles/network.txt
> cmd/text-install/helpfiles/network_manual.txt
> cmd/text-install/helpfiles/sparc_solaris_slices.txt
> cmd/text-install/helpfiles/sparc_solaris_slices_select.txt
> cmd/text-install/helpfiles/summary.txt
> cmd/text-install/helpfiles/timezone.txt
> cmd/text-install/helpfiles/users.txt
> cmd/text-install/helpfiles/welcome.txt
> cmd/text-install/helpfiles/x86_fdisk_partitions.txt
> cmd/text-install/helpfiles/x86_fdisk_partitions_select.txt
> cmd/text-install/helpfiles/x86_fdisk_slices_select.txt
> cmd/text-install/osol_install/__init__.py**
> 
> * The help text files have been reviewed separately.
> ** This file is not packaged, as osol_install/__init__.py is delivered
> by SUNWinstall. It exists so that the UI can be run directly from the
> workspace.
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to