Richard Lowe wrote: > Nathan Bush <nathan.bush at sun.com> writes: > >> Richard Lowe wrote: >>> Nathan Bush <nathan.bush at sun.com> writes: >>> >>>> Hi, >>>> >>>> Please review: >>>> http://cr.opensolaris.org/~nbush/scm-migration/483/webrev/ >>>> >>>> This change should address all of the following bugs: >>>> 465 webrev permission display fails for files in workspace root. >>>> 470 noise in webrev log for deleted files >>>> 471 "ERROR: set mode of" errors from webrev >>>> 483 webrev permission display change #372 has poor performance >>>> >>> 2271/2509: If the filename were of the form /tmp/$$.* this would happen >>> anyway (and would be done in a trap that dealt with most >>> signals, too). >>> >>> Same comment for SEDFILE, though less worthwhile there, I guess. >> Good point, especially regarding the trap. Here's a revised version: >> http://cr.opensolaris.org/~nbush/scm-migration/483/webrev.v2/ > > I'm happy with that, the only other thing I could suggest is to > perhaps comment that the pattern includes ' . ' to deal with symlinks, > if you choose to do that, I don't think we'll need another webrev with > just that change.
OK, I'll add this. >> I think I'd like it best if webrev created a private temporary directory >> with mktemp, then put all of its temporary files inside there, but that >> could be addressed later in a separate bug. > > I'd prefer that too, but agree, later. I'll file a bug for this so that it's not forgotten. Thanks for the review. I'm getting ready to push it now. --Nathan
