James Carlson wrote: > Nathan Bush writes: >> James Carlson wrote: >>> Nathan Bush writes: >>>> Proposed fix: >>>> http://cr.opensolaris.org/~nbush/scm-migration/372/webrev/ >>> 1525-1531,1596-1603: nit: indenting seems to get a little strange >>> around here. >> The preferred indenting style is not clear to me in these cases. >> I tried to follow a rule of tab characters only, except for four >> spaces on continuation lines. Can you suggest the proper changes? > > When you've got an embedded script (as with the Perl code on lines > 1525-1531, I'd suggest treating it as a program block (indented by > tab) rather than as one big continuation line. > > Just a nit, though.
OK, good point. I also changed the embedded sed script. >>> 1548-1551: I thought 'cp' preserved modes ... what is this for? > [...] >> So the purpose of this is to always copy the working file contents >> (and implicitly, permissions) into the webrev, but if it's also already >> under version control, apply the permissions of the SCCS/s.file. Then >> normalize the permissions in the webrev with "chmod u-w,go=u". > > Ah, ok. I get it now. Thanks. (Could have a comment saying > "propagate permissions the same way SCCS does" or some such, but no > big deal.) I added a block comment on the whole permission algorithm in the function. > Looks better -- still some nits about strange indenting (compare lines > 1604 and 1609, or 1620 and 1624). Fixed these too. New URLs: http://cr.opensolaris.org/~nbush/scm-migration/372/webrev.v3/ http://cr.opensolaris.org/~nbush/scm-migration/372/sample.v3/ % diff webrev.v[23]/raw_files/new/usr/src/tools/scripts/webrev.sh 1525,1531c1525,1532 < if (@stat = stat($ARGV[0])) { < $mode = $stat[2] & 0777; < printf "%03o\n", $mode; < exit 0; < } else { < exit 1; < }' $1 --- > if (@stat = stat($ARGV[0])) { > $mode = $stat[2] & 0777; > printf "%03o\n", $mode; > exit 0; > } else { > exit 1; > } > ' $1 1542a1544,1553 > # The following two sections propagate file permissions the > # same way SCCS does. If the file is already under version > # control, always use permissions from the SCCS/s.file. If > # the file is not under SCCS control, use permissions from the > # working copy. In all cases, the file copied to the webrev > # is set to read only, and group/other permissions are set to > # match those of the file owner. This way, even if the file > # is currently checked out, the webrev will display the final > # permissions that would result after check in. > 1597,1603c1608,1615 < 2,3 { < s/^deleted file /old / < s/^new file /new / < s/ mode 100/ / < /^old /p < /^new /p < }' | --- > 2,3 { > s/^deleted file /old / > s/^new file /new / > s/ mode 100/ / > /^old /p > /^new /p > } > ' | 1609c1621 < done --- > done 1624c1636 < done --- > done Thanks, --Nathan
