Sue,
The changes look good.
- Sundar
Susan Sohn wrote:
> Sundar,
>
> Thank you for the review. Responses below.
>
> Sundar Yamunachari wrote:
>> Susan Sohn wrote:
>>> Please review the changes for:
>>>
>>> 4194 need to make installadm tool changes for SPARC
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4194
>>>
>>> which are posted at:
>>>
>>> http://cr.opensolaris.org/~sohn/4194
>>>
>>> Thanks,
>>> Sue
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>
>> Sue,
>>
>> *create-client.sh: *
>> 288, 418: It is better to display something like "Setting up
>> SPARC/X86 client" to indicate which type of client being setup.
>
> Done.
>
>> *installadm-common.sh:*
>> Is the release information used only for X86?
>
> Currently it is used only for x86, though the file should be in the
> image for
> both sparc/x86.
>
>> installadm.h:*
>> 56: HTTP_PORT is defined here and installadm_common.sh. Can you
>> combine these two and define in one place?
>
> As we discussed, the best way to handle this would be for the code not
> to have any hard-coded values, but to get the value from the
> /var/ai/ai-httpd.conf file. I will write a bug to change the code to
> do this.
>
>> 115: The error message says "install image" where as In
>> installadm-common.sh (97) and create-client.sh(243), error message
>> says "OpenSolaris Install image". Can you make this as OpenSolaris
>> Install image?
>
> Done.
>
>> setup-sparc.sh:*
>> 134: Check the existence of ${WANBOOTCGI} before copy
>> 134: Check whether the target directory exists before copy
>
> Added checks and error msgs for both, exiting if there is a problem
> (though the
> target directory should always be there as it is installed by the
> SUNWinstalladm-tools package).
>
> webrev is updated, same location.
>
> Sue
>
>> - Sundar
>>
>
>
