Darren,
Thanks for reviewing. Responses below.
On 02/28/12 11:54, Darren Kenny wrote:
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?
As per our discussion just now, it seems gnome-open
might not currently be as reliable, so I'll leave it as is.
- Generally, is there really any use-case where we wouldn't want to have a
URI specified for opening?
Possibly not, but it seemed like an easy and obvious option
to add. Would you prefer it be removed?
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?
I did ask this question and have not heard back, so
I went ahead and presumed it should be localized.
Yes - the user will see it - it is the default, pre-populated
value for the Email field. Also, it will not matter if the
actual value if different in different locales - all
non-authorized email address are treated similarly.
- 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:
...
It works correctly as is, but I will add a comment
to clarify that 'no_proxy_chosen', 'proxy_chosen'
and 'aggregation_chosen' are mutually exclusive -
only one of them can be True. (I found the coding
easier and clearer if I used 3 booleans rather than
one enumerate var for these fields.)
- 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.
Yes. Will change as suggested.
- 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.
This was copied from some previous code I wrote
for 4xIP fields, where it was decided to auto-advance
when a third digit is entered in each field. I agree it's
not intuitive here and will remove.
Thanks,
- Dermot
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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
