Hi Ginnie.

Thanks for your feedback.

I'll tackle all of the non-GUI items...

On 02/28/12 15:15, Virginia Wray wrote:
Hi Jack --

Here is some feedback.

thanks,
ginnie


gui-install/src/base_screen.py
--------------------------
needs a copyright update

------------------------------


gui-install/src/gui_install_common.py
--------------------------------
lns 259 - run method - It isn't clear in this method what is the difference in
the two cases. What is the purpose of running firefox w/o a uri?

lns 272-273 - Need to address the log comment
-------------------------------------------

gui-install/src/screen_manager.py
---------------------------------------
ln 224 - seems like a stray comma at the end of the dictionary
----------------------------------------

system-config/helpfiles/Makefile
-----------------------------
needs a copyright update
Thanks.  Fixed.
-----------------------------------

system-config/profile/support_info.py
----------------------------------
ln 42 - you don't need really need a function to use the LOGGER.
You can initiate the logger once and then just use LOGGER.debug(message).
Support_info.py is just calling the logger the same way as other files are in this directory: network_info.y, nameservice_info.py, system_info.py. I'd like to be consistent.

lns 104 - 115 - do these need to be removed?
Removed.

lns 390 - 291 - does this need to be removed?
This is used for testing. There's more it controls below it. I'll remove before I push.

ln 427 - This comment seems a little cryptic. Could you reword it?
OK.  Changed to:
"If hub field is blank, just use direct mode (no proxy, no hub)."

ln 436 - 437 - Remove?
Same as for 390-391.  Will remove before I push.

ln 453-454, ln 470 - 471 - Remove?
These are the things 436-437 control.  Will remove before I push.

ln 613 - 616 - Move to where other class variables are defined.
Moved to module scope.
-------------------------------------------------------

support.py
-----------------------------------------------
ln 362 - typo "servcr"
Thanks.  Fixed.

ln 528 - 529 - is there a reason that the result has to be stored in self.current_choice?
or could it go directly into self.support .netcfg?
self.current_choice holds a local user-selected value that can change. self.support.netcfg is part of the DOC, and is stored when the user moves on to the next screen. The protocol I've adhered to is to update the DOC copy of a field when the user moves on to the next screen.

ln 888 - 891, ln 1188 -- Looks like a lot of parens. Just curious...are they all necessary or is it personal style?
Two out of three levels/sets are required. I could get rid of the innermost level, but I think it makes the code clearer.
------------------------------------------------------------

text_install/summary.py
--------------------------------
needs a copyright update
Fixed.

CDDL missing?
Fixed.
--------------------------

system-install-gui-install.mf
-------------------------
needs copyright update
Fixed.

    Thanks again,
    Jack
On 2/27/12 11:00 PM, 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

Reply via email to