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
