Joe,
Many thanks for volunteering to look at this. I know we're pushing to
get a lot out of you in your last week, and I appreciate it (I say that
with both my SCI hat on and with my ISIM hat on).
On 01/10/11 01:31 PM, [email protected] wrote:
On 01/ 6/11 07: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
This looks great Keith.
Here is what little I could find.
Hope this helps.
Joe
- - -
General comment(s).
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Issue:
------
All the copyrights should be updated to 2011
usr/src/cmd/system-config/date_time.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Nit/question:
-------------
143 LOGGER.debug("now year month day hour minute: %s %s %s %s %s",
Does the word "now" need to be there? This reads oddly to me.
I would suggest:
LOGGER.debug("year month day hour minute: %s %s %s %s %s",
or
LOGGER.debug("current year month day hour minute: %s %s %s %s %s",
Seems like it would be reasonable to update those debug statements, yes.
usr/src/cmd/system-config/helpfiles/Makefile
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
looks OK
usr/src/cmd/system-config/helpfiles/date_time.txt
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
looks OK
usr/src/cmd/system-config/helpfiles/network.txt
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
looks OK
usr/src/cmd/system-config/helpfiles/network_manual.txt
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
looks OK
usr/src/cmd/system-config/helpfiles/summary.txt
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
This file contains only the one line:
"DUMMY TEXT"
DUMMY TEXT explains that screen pretty well, no?
I'll copy/update the text from the text install's summary.txt file, and
have Barbara take a look when she has a moment.
usr/src/cmd/system-config/helpfiles/timezone.txt
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Nit:
This series of screens enable you to type the correct time zone for the
system to be installed.
This just reads oddly to me.
This just reads more gooderer to me:
This series of screens allows one to enter the correct time zone for the
system being installed.
I'll leave this nit up to you. Please just reread it to make sure it
sounds good to you.
I'll ask Barbara to take a look at it. The contents of these files is
actually her domain.
usr/src/cmd/system-config/helpfiles/users.txt
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
what are the restrictions for root and user password?
AFAIK there are none.
usr/src/cmd/system-config/helpfiles/welcome.txt
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
This file contains only the one line:
"DUMMY TEXT"
Same as with the other.
usr/src/cmd/system-config/network_nic_configure.py
usr/src/cmd/system-config/network_nic_select.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Issue/Confirm:
--------------
71 HELP_DATA = ("/usr/share/sysconfig/help/%s/network_manual.txt",
72 _("Manually Configure: NIC"))
73 HELP_FORMAT = " %s"
HELP_DATA and HELP_FORMAT are not used locally. Are they picked up
someplace else? Please confirm these are needed.
If so maybe a comment would help...
These override values from the parent class, and are used by
help_screen.py, and so the explanatory comments are in those locations.
Issue/Confirm:
--------------
What network .txt file belong with what source file? Both
network_nic_select
and network_nic_configure point to:
HELP_DATA = ("/usr/share/sysconfig/help/%s/network_manual.txt",
Is this correct?
Yes, this is correct. The text in that file explains the purpose and
functionality of both screens. (network_nic_select is a simple list of
all discovered NICs on the system; as there was not much to add on that
screen, its help text was combined with the following screen,
network_nic_configure. This dates back to the original text installer
push, though those screens were not enabled then).
usr/src/cmd/system-config/network_type.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Looks OK
usr/src/cmd/system-config/summary.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Looks OK
usr/src/cmd/system-config/timezone.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Looks OK
usr/src/cmd/system-config/users.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Question:
---------
265 '''Check for mismatched passwords, bad login names, etc.'''
Why remove this comment? Seems valid.
I'll restore it. It was probably deleted accidentally.
usr/src/cmd/system-config/welcome.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Looks OK
usr/src/cmd/system-config/sysconfig.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Issue:
------
67 usage = "usage: %prog [-l FILE] [-v LEVEL] [-n]"
-b is not listed in the usage
Will update.
usr/src/cmd/system-config/test/test_users.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Looks OK
usr/src/pkg/manifests/system-install-configuration.mf
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Seems OK
usr/src/cmd/system-config/__init__.py
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Nit:
----
37 from solaris_install.sysconfig.profile import ConfigProfile
Not in correct alphabetical order.
Thanks, we'll fix that.
usr/src/cmd/system-config/Makefile
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Seems OK
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
