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. > > 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.) > In fact, it looks like there is no way to change the permissions of > an existing file in a parent workspace via a putback. I think the > only way to do this is via a chmod of the SCCS/s.file directly in the > parent. So it may be somewhat misleading to print messages in a webrev > that say it contains a permission "change". But, I think it's still > useful to flag this condition even if it cannot be propagated to the > parent. It indicates that the child workspace owner must have done > an improper manual chmod of their SCCS/s.file, and they might have > come to depend on the effects of this in other ways, such as by > forgetting a makefile rule to do a chmod explicitly. Yes; if it happens, it's very likely an error. > New URLs: > http://cr.opensolaris.org/~nbush/scm-migration/372/webrev.v2/ Looks better -- still some nits about strange indenting (compare lines 1604 and 1609, or 1620 and 1624). -- James Carlson, Solaris Networking <james.d.carlson at sun.com> Sun Microsystems / 35 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677
