Hi Sue, Looks good.
Alok On Tue, 2 Dec 2008, Susan Sohn wrote: > Hi Alok, > > Thank you for the review. See below. > > Alok Aggarwal wrote: >> Hi Sue, >> >> On Mon, 1 Dec 2008, Sue Sohn wrote: >> >>> Please review the changes for: >>> >>> 4111 having multiple install services all said "Opensolaris" in grub menu >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4111 >>> >>> 4553 create-client returning non zero exit code. >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4553 >>> >>> 4559 create-client: succeeded when invalid mac addr was given. >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4559 >>> >>> 4819 add/remove/delete-client: getting full usage message >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4819 >>> >>> 4534 create-service: target directory is created upon failure with >>> non-existent >>> or invalid image. >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4534 >>> >>> >>> which are posted at: >>> >>> http://cr.opensolaris.org/~sohn/4111_4553_4559_4819_4534 >> >> ai_bootroot_configure: lines 43-51: need to update the comment >> to account for the changes you're making > > Thanks, I've updated them. > >> installadm-common.sh: line 118-119: should not use $dir here, >> depending upon the context this could be wrong (if another >> function were to start calling get_relinfo). It would be >> better to set say releasepath=$1/.release and use that instead. > > Fixed. > >> installadm-common.sh: lines 119: do you really want to display >> a version such as - >> >> OpenSolaris2008.11snv_101X86 >> >> ? > > It will not look that way - double spaces are removed, not single spaces. > So it essentially strips the leading spaces. > >> That doesn't look too pleasing. What is $VERSION if the .release >> file doesn't exist? > > VERSION will be "OpenSolaris" in that case (defined at the top of > the file). > >> create-client.sh: lines 246-248: This will change when defect 5053 >> is addressed but in the interim, maybe the following error message >> would be more appropriate? >> >> echo "${myname}: ${IMAGE_PATH}/solaris.zlib does not exist. " >> "The image provided is not an OpenSolaris image" > > Fixed, with this wording for the second line: > "The specified image is not an OpenSolaris image." > > > Please see the updated webrev, same location. > > Sue > > > >
