William Schumann wrote:
> Updated webrev including all requested changes with Bing's changes
> included.
>
> http://defect.opensolaris.org/bz/show_bug.cgi?id=6590
> http://cr.opensolaris.org/~wmsch/bug-6590/
> previous webrev: http://cr.opensolaris.org/~wmsch/bug-6590/oldwebrev
>
> Thank you,
> William
William,
There are two small issues/nits I had pointed out in my code review
feedback which you had not addressed.
See below.
How do you want to address these?
I do not require anyother webrev.
Joe
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
usr/src/cmd/auto-install/auto_parse.c 38 lines changed
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Nit: 187-> 224
-----------------------
Please add a CR above the consecutive ai_get_manifest_element_value()
invocations for readibility?
I found this a little hard to read and noticed the <CR> above
calls to ai_get_manifest_element_value elsewhere in the file
improved readibilty.
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
usr/src/lib/libict_pymod/ict.py 60 lines changed
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Issue:
-----------------------
1277 if status == 0:
if status == 0 is backwards from how this is handled throughout.
and add logging message before return ICT_SVCCFG_FAILURE
Suggestion:
1277 if status == 0:
1278 return 0
1279 return ICT_SVCCFG_FAILURE
Change to:
if status != 0:
prerror('Unexpected error issuing svccfg -f command.')
prerror('Failure. Returning: ICT_SVCCFG_FAILURE')
return ICT_SVCCFG_FAILURE
return 0
If you don't want to rearrange this to be more consistent with other ICT
code please issue:
prerror('Unexpected error issuing svccfg -f command.')
prerror('Failure. Returning: ICT_SVCCFG_FAILURE')
Prior to:
1279 return ICT_SVCCFG_FAILURE
