Roland Mainz wrote: > Sarah Jelinek wrote: > >> Located at: >> >> http://cr.opensolaris.org/~sjelinek/slim/ >> >> Are the changes to modify the grub menu to say 'OpenSolaris', not >> 'Solaris Developer...' >> > > >> +# Fix up the grub entry. This is required because bootadm 'assumes' >> +# Solaris. And, even though /etc/release says OpenSolaris it truncates >> +# the 'Open' off. Replace this globally. >> + >> +/bin/sed -e 's/title Solaris/title OpenSolaris/g' $MENUFILE > $MENUFILE.new >> +if [ $? -eq 0 ]; then >> > > Please use arithmetric operators when comparing numbers, e.g. this > should be... > -- snip -- > if (( $? == 0 )); then > -- snip -- > .. otherwise the shell will expand "$?" to a string, then "0" is > converted to a string, then the "test" builtin is called which converts > both arguemnts back to numbers... which is an overkill... > > >> + cat $MENUFILE.new > $MENUFILE >> + rm $MENUFILE.new >> > > Please quote this, e.g. > -- snip -- > cat "${MENUFILE}.new" > $MENUFILE > rm "${MENUFILE}.new" > Ok, will fix. > -- snip -- > I also added '{'+'}' to avoid the ambigous case where '.' may be a valid > part of the variable name (in which case you would use the variable > "MENUFILE.new" while the intended variable was "MENUFILE" ... > > >> +fi >> + >> > > BTW: Is anyone accepting patches for this script ? I took a quick look > at > http://cr.opensolaris.org/~sjelinek/slim/packages/utils/install-finish.html > and still suffering from what I saw there (well, you may know the > rant... use [[ ]] instead of [ ], use arithmetric operators when > comparing numbers, please set a propper PATH at the beginning of the > script, use "function foo", not "foo()" in ksh scripts, use propper exit > codes etc. etc) ... ;-( > > Sure... we are accepting patches to this script. It is open so feel free.
Thanks for the review. sarah ***** > ---- > > Bye, > Roland > >
