Sebastien Roy wrote: > > I'm getting ready to putback "EOF of Mobile IP" to Nevada. I made one > change after the code-review completed that needs to be reviewed prior to > filing the RTI, and that is the change to bfu.sh to remove the obsolete > SUNWmipr and SUNWmipu package contents. > > The webrev is in the same location as before: > > http://cr.opensolaris.org/~seb/rm_mobileip_webrev/ > > I only need bfu.sh reviewed; everything else was reviewed in the previous > round of code-reviews.
Some nitpicking based on http://www.opensolaris.org/os/project/shell/shellstyle/ ("bfu.sh" is a ksh script, follow the colored tags labelled with "ksh" in that document): - Plese use "if [[ ... ]]" instead of "if [ ... ]", e.g. turn -- snip -- + if [ -d $rootprefix/var/sadm/pkg/SUNWmipr ]; then -- snip -- into + if [[ -d $rootprefix/var/sadm/pkg/SUNWmipr ]]; then -- snip -- - Nitpicking: Please use "function foo" functions, not Bourne-style "foo()", e.g. turn -- snip -- +remove_eof_mobileip() { + typeset -r mip_pkgs='SUNWmipr SUNWmipu' + typeset pkg -- snip -- into -- snip -- +function remove_eof_mobileip { + typeset -r mip_pkgs='SUNWmipr SUNWmipu' + typeset pkg -- snip -- (see http://www.opensolaris.org/os/project/shell/shellstyle/#use_ksh_style_function_syntax for an explanation of the background) - It may be usefull to put quotes around the "rm" calls to avoid that "rm" may run amok when ${rootprefix} returns garbage (e.g. contains spaces etc.), e.g. turn -- snip -- + rm $rootprefix/etc/inet/mipagent.conf* + rm $rootprefix/etc/init.d/mipagent + rm $rootprefix/etc/rc0.d/K06mipagent + rm $rootprefix/etc/rc1.d/K06mipagent + rm $rootprefix/etc/rc2.d/K06mipagent + rm $rootprefix/etc/rc3.d/S80mipagent + rm $rootprefix/etc/rcS.d/K06mipagent + rm $rootprefix/etc/snmp/conf/mipagent* -- snip -- into -- snip -- + rm "${rootprefix}/etc/inet/mipagent.conf"* + rm "${rootprefix}/etc/init.d/mipagent" + rm "${rootprefix}/etc/rc0.d/K06mipagent" + rm "${rootprefix}/etc/rc1.d/K06mipagent" + rm "${rootprefix}/etc/rc2.d/K06mipagent" + rm "${rootprefix}/etc/rc3.d/S80mipagent" + rm "${rootprefix}/etc/rcS.d/K06mipagent" + rm "${rootprefix}/etc/snmp/conf/mipagent"* -- snip -- - Just curious: What's the reason for making some variables like "mip_pkgs" read-only ("typeset -r ..."), e.g. -- snip -- + typeset -r mip_pkgs='SUNWmipr SUNWmipu' -- snip -- ? ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) [EMAIL PROTECTED] \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 7950090 (;O/ \/ \O;) _______________________________________________ networking-discuss mailing list [email protected]
