John Zolnowsky x69422/408-404-5064 wrote: > > I'm seeking codereview for my changes to relieve a minor annoyance. > > The webrev is at > http://cr.opensolaris.org/~johnz/webrev.6738408/ > > The big insertion and deletion are just the relocation of the otherwise > unmodified which_scm tests to immediately after the determination of > the workspace and parent workspace paths.
>From http://cr.opensolaris.org/~johnz/webrev.6738408/on-jzx.patch (patch code is quoted with "> " and yes... I know you're just coping the code around but the cleanup would be nice): > +# Echo the SCM types of $CODEMGR_WS and $BRINGOVER_WS > +function wstypes { > + typeset parent_type child_type > + Please define the variable "junk" as local variable. > + env CODEMGR_WS=$BRINGOVER_WS $WHICH_SCM 2>/dev/null | read parent_type > junk Please change this to: -- snip -- CODEMGR_WS="$BRINGOVER_WS" "$WHICH_SCM" 2>/dev/null | read parent_type junk -- snip -- e.g. add double-quotes and the "env" is AFAIK not neccesary. [snip] > + > + echo $child_type $parent_type > +} Please use: -- snip -- printf "%s %s\n" "$child_type" "$parent_type" -- snip -- (see http://www.opensolaris.org/os/project/shell/shellstyle/#avoid_echo) ... and at the end of the function "wstypes" a "return 0" is missing (basically http://www.opensolaris.org/os/project/shell/shellstyle/#use_proper_exit_code). ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) roland.mainz at nrubsig.org \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 3992797 (;O/ \/ \O;)
