On 01/12/11 10:27 AM, Karen Tung wrote:
Hi Keith and Jan,
Everything in part [d] are reviewed. The changes look good,
I only have a few minor comments and questions.
[d] terminalui Python module
-------------------------
usr/src/lib/terminalui/base_screen.py
-------------------------
- line 82-86: question: since CONFIRM_QUIT_HEADER, QUIT_TEXT,
CANCEL_BUTTON
and CONFIRM_BUTTON are all required values. Why not make them as require
arguments for the __init__() function?
Two reasons:
1) They only really need to be set once per application, rather than
once per screen object in the application - it should be the same for
all screens.
2) The context in which the screens are created is not always a context
which knows what the values should be. For example, when the Text
Installer builds up the screen objects, the sysconfig module actually
handles building half the screens. The sysconfig module doesn't (and
shouldn't) know what the values should be for that execution context.
- line 115: question: since install_profile is no longer used, why not
remove it?
Agreed, that should get removed. It was temporarily changed during
development to smooth the transition.
- line 34-38: please alphabetize all the imports.
Will do.
--------------------------------
usr/src/lib/terminalui/help_screen.py
--------------------------------
- line 241-236: question, can this be done with a "with" block so you
don't need to explicitly close the file at line 263?
Sounds reasonable.
--------------------------------
usr/src/lib/terminalui/main_window.py
--------------------------------
- line 35: Nit: move this after 36 to keep the imports alphabetized.
Ok
--------------------------------
usr/src/lib/terminalui/test/test_scroll_window.py
--------------------------------
- lines 36-39: alphabetize imports
Will do. Thanks for the feedback!
- Keith
Thanks,
--Karen
On 01/ 6/11 04:10 PM, Keith Mitchell wrote:
Hi Jean, Joe, Karen,
Thanks for volunteering to review the SCI code. The webrev is at:
http://cr.opensolaris.org/~kemitche/sci.1/
Jean/Joe - you agreed to look at part [a]. Karen, part [d]. Again,
it's mostly moved files and updated imports.
- Keith
[a] sysconfig module - UI part
usr/src/cmd/system-config/date_time.py
usr/src/cmd/system-config/helpfiles/Makefile
usr/src/cmd/system-config/helpfiles/date_time.txt
usr/src/cmd/system-config/helpfiles/network.txt
usr/src/cmd/system-config/helpfiles/network_manual.txt
usr/src/cmd/system-config/helpfiles/summary.txt
usr/src/cmd/system-config/helpfiles/timezone.txt
usr/src/cmd/system-config/helpfiles/users.txt
usr/src/cmd/system-config/helpfiles/welcome.txt
usr/src/cmd/system-config/network_nic_configure.py
usr/src/cmd/system-config/network_nic_select.py
usr/src/cmd/system-config/network_type.py
usr/src/cmd/system-config/summary.py
usr/src/cmd/system-config/timezone.py
usr/src/cmd/system-config/users.py
usr/src/cmd/system-config/welcome.py
usr/src/cmd/system-config/sysconfig.py
usr/src/cmd/system-config/test/test_users.py
usr/src/pkg/manifests/system-install-configuration.mf
usr/src/cmd/system-config/__init__.py
usr/src/cmd/system-config/Makefile
[d] terminalui Python module
usr/src/lib/Makefile.targ
usr/src/lib/terminalui/Makefile
usr/src/lib/terminalui/__init__.py
usr/src/lib/terminalui/action.py
usr/src/lib/terminalui/base_screen.py
usr/src/lib/terminalui/color_theme.py
usr/src/lib/terminalui/edit_field.py
usr/src/lib/terminalui/error_window.py
usr/src/lib/terminalui/help_screen.py
usr/src/lib/terminalui/i18n.py
usr/src/lib/terminalui/inner_window.py
usr/src/lib/terminalui/list_item.py
usr/src/lib/terminalui/main_window.py
usr/src/lib/terminalui/screen_list.py
usr/src/lib/terminalui/scroll_window.py
usr/src/lib/terminalui/test/test_edit_field.py
usr/src/lib/terminalui/test/test_i18n.py
usr/src/lib/terminalui/test/test_scroll_window.py
usr/src/lib/terminalui/test/test_window_area.py
usr/src/lib/terminalui/window_area.py
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
