"Mark J. Nelson" wrote: > > I reckon the interest list for the webrev bug is as good a place as any to > seek code review. I'm not an HTML guy, so I did a pretty generic > seek-and-replace for ;<\([^ ]+\) \(.*\)/>;<\1 \2></\1>; in the webrev > source. > > The two different webrevs are > > http://cr.opensolaris.org/~mjnelson/webrev.481/ > http://cr.opensolaris.org/~mjnelson/webrev.481-firefox3.0/ >
1. I assume it may be better to defer any nightly.sh cleanups until your putback is done, right ? 2. Quick look at http://cr.opensolaris.org/~mjnelson/webrev.481/scm-fixes.patch (patch code is quoted with "> "): > > - [ $? -eq 0 ] || build_ok=n > + if [ $? -ne 0 ]; then Please use ... -- snip -- if (( $? != 0 )) ; then -- snip -- (see http://www.opensolaris.org/os/project/shell/shellstyle/#compare_exit_code_using_math) > + echo "cpio failed for $DEST" | Please use -- snip -- printf "cpio failed for %s\n" "$DEST" -- snip -- .. to prevent that "echo" interprets any special characters in $DEST > + tee -a $mail_msg_file >> $LOGFILE > + build_ok=n > + fi > ;; > mercurial) > - copy_source_mercurial $DEST $srcroot > - [ $? -eq 0 ] || build_ok=n > + copy_source_mercurial $DEST $srcroot || build_ok=n > ;; > *) > build_ok=n > @@ -358,7 +365,11 @@ > > if [[ -n "$open_top" ]]; then > hg locate "$open_top" | cpio -pd "$dest" >>$LOGFILE 2>&1 > - [ $? -eq 0 ] || return 1 > + if [ $? -ne 0 ]; then Please use ... -- snip -- if (( $? != 0 )) ; then -- snip -- (see http://www.opensolaris.org/os/project/shell/shellstyle/#compare_exit_code_using_math) > + echo "can't copy $srcroot: closed tree not" \ Please use -- snip -- printf "can't copy %s: closed tree not present\n" "$srcroot" -- snip -- ... and please try to avoid breaking string literals at the 80 char limit - this doesn't work properly in shellscripts. $ echo "foo foo" # is not equal to $ echo "foo" "foo" # > + "present." | tee -a $mail_msg_file >> $LOGFILE > + return 1 > + fi > fi > > if [[ -n "$closed_top" ]]; then > @@ -366,13 +377,23 @@ > if [[ "$closed_top" = usr/closed ]]; then > (cd usr/closed; hg locate | > cpio -pd "$dest/usr/closed") >>$LOGFILE 2>&1 > + if [ $? -ne 0 ]; then Please use ... -- snip -- if (( $? != 0 )) ; then -- snip -- (see http://www.opensolaris.org/os/project/shell/shellstyle/#compare_exit_code_using_math) > + echo "cpio failed for $dest/usr/closed" | Please use -- snip -- printf "cpio failed for %s/usr/closed\n" "$dest" -- snip -- > + tee -a $mail_msg_file >> $LOGFILE > + return 1 > + fi > else > # copy subtree of usr/closed > closed_top=${closed_top#usr/closed/} > (cd usr/closed; hg locate "$closed_top" | > cpio -pd "$dest/usr/closed") >>$LOGFILE 2>&1 > + if [ $? -ne 0 ]; then Please use ... -- snip -- if (( $? != 0 )) ; then -- snip -- (see http://www.opensolaris.org/os/project/shell/shellstyle/#compare_exit_code_using_math) > + echo "cpio failed for " \ > + "$dest/usr/closed/$closed_top" | Please use -- snip -- printf "cpio failed for %s\n" "$dest/usr/closed/$closed_top" -- snip -- > + tee -a $mail_msg_file >> $LOGFILE > + return 1 > + fi > ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) roland.mainz at nrubsig.org \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL <currently fluctuating> (;O/ \/ \O;)
