Hi Ginnie,

Thanks for reviewing.  Responses to gui-install/* comments
below.

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

Here is some feedback.

thanks,
ginnie


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

fixed.


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


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?

There probably is any call for starting browser without a uri
at present, but it seemed an obvious feature to add and
only costs one extra line of code.  I've amended the comment
at 263 to say:
            - uri, URI to open in browser, or None, if you
              just wish to start the browser.
which I hope clarifies it a bit more.

lns 272-273 - Need to address the log comment

Fixed (by adding a log command).  Thanks.

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

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

Having a redundant comma after the last element in a
dictionary  is explicitly allowed in Python and not an error.
It just makes it a little easier adding additional objects later on.

Thanks,
- Dermot


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

system-config/helpfiles/Makefile
-----------------------------
needs a copyright update
-----------------------------------

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).

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

lns 390 - 291 - does this need to be removed?

ln 427 - This comment seems a little cryptic. Could you reword it?

ln 436 - 437 - Remove?

ln 453-454, ln 470 - 471 - Remove?

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

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

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?

ln 888 - 891, ln 1188 -- Looks like a lot of parens. Just curious...are they all necessary or is it personal style?
------------------------------------------------------------

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

CDDL missing?
--------------------------

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




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:

    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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to