jeanm wrote: > Joseph J VLcek wrote: >> jeanm wrote: >>> Could I get a code review (Alok and Jan preferred) for the following: >>> >>> 12612 <http://defect.opensolaris.org/bz/show_bug.cgi?id=12612> >>> get_service_with_global_scope() looks in the wrong place for the boot >>> archive >>> 12642 <http://defect.opensolaris.org/bz/show_bug.cgi?id=12642> >>> platform directory needs to only contain the necessary files >>> >>> >>> >>> >>> webrev: >>> >>> http://cr.opensolaris.org/~jeanm/slim_12612_12642/ >>> >>> If you are planning to look at this please let me know so I don't >>> push before you get back to me. >>> >>> >>> Thanks, >>> >>> jean >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >> >> >> Hey Jean, >> >> Looks good. A couple simple things. >> >> >> Issue 1: >> -------- >> >> 88 cd ${PKG_IMG_PATH}/platform >> >> Prior to cd-ing to a directory it is best to save the current working >> directory and cd back to that when done. >> >> e.g. >> >> initial_cwd=`pwd` >> 88 cd ${PKG_IMG_PATH}/platform >> ... >> cd ${initial_cwd} >> >> >> I thought it woudl be worth mentioning but this may not be necessary >> in this case... ? You decide. > I'll need to look at the context. Heaven forbid we actually rely upon > the cd having happened. >> >> >> Issue 2: >> -------- >> >> 89 if [ `uname -p` == "sparc" ] ; then >> >> uname should have a full path qualifier or PATH should be defined. > Will do. >> >> See "12 Pathnames" here: >> http://hub.opensolaris.org/bin/view/Community+Group+on/shellstyle >> >> usr/src/cmd/installadm/setup-sparc.sh >> ------------------------------------- >> >> 114 image_directory=`/usr/bin/dirname "$image_directory"` >> 115 image_directory=`/usr/bin/dirname "$image_directory"` >> >> Lines 114 and 115 are the same. > > I'll add the comment as your other email stated. > > Jean >> >> Joe >
Thank you. Joe
