Hi Jack/Dermot. I focused on the GUI installer side (apart from the whole support.xml file) of things, here are my comments from this:
gui_install_common.py: - Lines 90 and 259 - 273: NIT: This makes use of firefox directly, it would probably be more correct to use the 'gnome-open' application since this would allow for things to be more agnostic should anything change later? - Generally, is there really any use-case where we wouldn't want to have a URI specified for opening? support_screen.py: - Line 60: Do you really expect the [email protected] e-mail address to be localized? Is it even visible to the user? - Lines 363 - 386: Is there not a possibility that any changes to aggregation_chosen or its sub-elements is missed if the user had originally configured a proxy? Just seems like some of the elifs should be extracted from this list to a separate: if not any_changes: if aggregation_chosen: ... - Lines 559 - 589: Why do we limit to 5 characters? Just wondering, but would seem more correct to limit to at most a value of 65535 rather than 5 characters. - Lines 581 - 584: I'm not sure that I like the idea of automatically jumping to the next field on entering the 5th character - could be quite confusing for people. Thanks, Darren. On 28/02/2012 06:00, Jack Schwartz wrote: > Hi everyone. > > This is a webrev for the OCM/ASR configuration project. > > OCM and ASR are services which send limited system configuration and fault > information to Oracle, to offer better customer support. This project > provides installer and sysconfig support for configuring the OCM and ASR > services. > > There are two parts to this project: > text screens: > - usr/src/cmd/system-config/Makefile > - usr/src/cmd/system-config/__init__.py > - usr/src/cmd/system-config/helpfiles/Makefile > - usr/src/cmd/system-config/profile/Makefile > - usr/src/cmd/system-config/profile/__init__.py > - usr/src/cmd/system-config/profile/support_info.py > - usr/src/cmd/system-config/summary.py > - usr/src/cmd/system-config/support.py > - usr/src/cmd/text-install/summary.py > - usr/src/pkg/manifests/system-install-configuration.mf > > GUI screens (courtesy of Dermot): > - usr/src/cmd/gui-install/aux/INSTALL_SUPPORT_PANEL.txt > - usr/src/cmd/gui-install/aux/Makefile > - usr/src/cmd/gui-install/src/Makefile > - usr/src/cmd/gui-install/src/base_screen.py > - usr/src/cmd/gui-install/src/gui_install_common.py > - usr/src/cmd/gui-install/src/help_dialog.py > - usr/src/cmd/gui-install/src/screen_manager.py > - usr/src/cmd/gui-install/src/support_screen.py > - usr/src/cmd/gui-install/xml/Makefile > - usr/src/cmd/gui-install/xml/gui-install.xml > - usr/src/cmd/gui-install/xml/support.xml > - usr/src/pkg/manifests/system-install-gui-install.mf > > Please review. Dermot and I would like people familiar with sysconfig > and/or text installer to review the text screens, and people familiar with > GUIs to review the GUI screens. Each part is about 2000 lines, so please > pick a section to review. > > Dermot and I would like this round of review to be completed by next Monday > 3/5 COB if possible. > > Webrev:https://cr.opensolaris.org/action/browse/caiman/schwartz/7104766_1 > > Thanks, > Jack and Dermot > > > _______________________________________________ > caiman-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
