New webrev at
http://cr.opensolaris.org/~bpytlik/ips-5211-v2/

Need one more set of eyes on this please.

Brock

Shawn Walker wrote:
> Brock Pytlik wrote:
>> Webrev:
>> http://cr.opensolaris.org/~bpytlik/ips-5211-v1/
>>
>> Bug:
>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5211
>> cfg_cache needs to be able to live elsewhere
>
> depot.py:
>   line 120: you need to add the new --repo-config-file option here; 
> i'm inclined to say this should just be "--cfg-file" instead of 
> --repo-cfg-file.
ok
>
>   lines 122-143:  I know this sucks, but please re-flow all of these 
> so it lines up with line 144.
no need now
>
>   line 144-145: This really isn't a path, it's the pathname of an 
> alternate configuration file to use instead of the default one.
fixed
>
> repository.py:
>   line 103: default seems an odd name for this attribute
changed to default_cfg_path
>
>   line 133: please don't use the numeric codes here; use errno.EPERM, 
> errno.EACCES, errno.EROFS; also, I wouldn't assign this to perm_code, 
> just use the set() right in the if statement and avoid the unnecessary 
> assignment
fixed
>
> Can you add a new test to ensure that the depot accepts this as a 
> command-line option, and add it to depotcontroller?
>

yep
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to