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.

> 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.

-- Rich

Reply via email to