All,

As of yet, we've not received any code review comments for the first 
round of review. I realize that there is a lot of code, so to facilitate 
reviewing, I've broken down the files into the subsections (see below 
for which files fall into which subsections). Reviewers may find it 
easier to select one or two subsections for review.

Installation Profile: Classes describing the installation
UI Components: Generic UI components building on the curses module
Screens: Python files describing the specific text installer screens
Text Install python core: The core files including the primary 
executable, and additional supporting modules (including UI components 
that are text installer specific, as opposed to generic).
Image/Distro-const: Files and components relating to building and 
running a Text Installer image
Library Changes: Changes to libtd (including libtarget_pymod, the libtd 
Python bridge)
Help Files: Help Screen text
Packaging/Makefiles: Package definitions, dependencies, makefiles, and 
so forth

As a reminder, the focus of the first round of code review is the UI 
(comprised of the "UI Components," "Screens," and "Text Install python 
core" subsections). The "Installation Profile" and "Library Changes" 
sections are also very relevant. The remaining sections are lower 
initial priority, but we still welcome comments on them.

If you plan on reviewing, please let me know which files/section(s) 
you'll be looking at, so we can keep track of which portions of the code 
still need explicit review. I know that there are reviewers looking at 
the following files:

base_screen.py
help_screen.py
network_nic_configure.py
network_nic_select.py
network_type.py
partition_edit_screen.py
screen_list.py
timezone_locations.py
timezone_regions.py
timezone.py

main_window.py
inner_window.py

edit_field.py

Note that there's nothing wrong with reviewing the files listed above. 
In particular, inner_window, main_window, disk_window and edit_field 
almost certainly deserve a second (third, fourth) set of eyes at some 
point before the final push.

Thanks,
Keith

-----

Breakdown of file groupings:

Installation Profile:
cmd/text-install/osol_install/profile/__init__.py
cmd/text-install/osol_install/profile/disk_info.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/system_info.py
cmd/text-install/osol_install/profile/user_info.py
cmd/text-install/osol_install/profile/zfs_info.py**

* As mentioned in a prior email, this file has changed significantly 
from the initial webrev. Please hold off on reviewing the file until the 
next webrev / code review.
**This file is unused and has been removed in later revisions.

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

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/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

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**

* This subclass of "InnerWindow" has enough knowledge of Text Installer 
specifics that it can't be classified as a 'generic' UI component
** This is the 'main' program

Image/Distro-const:
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_plat_setup.py
cmd/distro_const/text_install/tm_post_bootroot_pkg_image_mod
cmd/distro_const/text_install/tm_pre_bootroot_pkg_image_mod
cmd/distro_const/utils/create_iso
cmd/slim-install/svc/live-fs-root
cmd/text-install/svc/text-mode-menu.xml
cmd/text-install/text-mode-menu/text-mode-menu.ksh

Library Changes:
lib/libtarget_pymod/discover.c
lib/libtarget_pymod/disk.c
lib/libtarget_pymod/disk.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/libtd/td_api.h
lib/libtd/td_dd.c
lib/libtd/td_dd.h
lib/libtd/test_td.c
lib/libzoneinfo_pymod/libzoneinfo.c

Help Files:
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.txt
cmd/text-install/helpfiles/x86_fdisk_slices_select.txt

Other:
cmd/text-install/osol_install/__init__.py*
cmd/text-install/osol_install/text_install/td_dummy.py**

* 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.
** This file has been removed in later revisions and does not need 
reviewing.

Packaging/Makefiles: Package definitions, dependencies, makefiles, and 
so forth
Makefile.master
README
Targetdirs
cmd/Makefile
cmd/Makefile.targ
cmd/distro_const/Makefile
cmd/distro_const/text_install/Makefile
cmd/text-install/Makefile
cmd/text-install/helpfiles/Makefile
cmd/text-install/osol_install/profile/Makefile
cmd/text-install/osol_install/text_install/Makefile
cmd/text-install/svc/Makefile
cmd/text-install/text-mode-menu/Makefile
lib/Makefile
lib/libtarget_pymod/Makefile
lib/libzoneinfo_pymod/Makefile

Sue Sohn wrote:
> The Text Install project is requesting a code review of all code done 
> thus far.
>
> The webrev is located at:
>
> http://cr.opensolaris.org/~kemitche/text_v1
>
> We are holding a preliminary code review because the UI is mostly 
> complete and
> there is quite a lot of code to be reviewed. Although there will be 
> another code
> review later in the project, this will give people more time to review 
> the code
> and provide comments. Feel free to review all the files or a subset.
>
> Please send review comments by COB December 11, 2009.
>
> Keith will be holding a code walkthrough of the UI on Tuesday, 
> December 1 (time
> TBD). Interested parties should contact keith.mitchell at sun.com to 
> ensure proper
> coordination of the conference call details.
>
> Thanks,
> Sue
>

Reply via email to